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.

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

syscall/js: expose a scope argument in JS interop #56085

Closed
wants to merge 287 commits into from

Conversation

romaindoumenc
Copy link

Updates #56084

This exposes the scope object in Javascript Go constructor, and makes it available in syscall/js.

Not to be merged as such, this is the basis for a proposal.

@google-cla
Copy link

google-cla bot commented Oct 6, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

4a6f656c and others added 29 commits November 3, 2022 18:46
Combine masking with a negative value and zero extension into a single
AND operation.

Change-Id: I0b2a735b696d65568839fc4504445eeac3d869a6
Reviewed-on: https://go-review.googlesource.com/c/go/+/428498
Reviewed-by: Joedian Reid <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Joel Sing <[email protected]>
…scv64

Convert SLT/SLTU with a suitably valued constant into a SLTI/SLTIU instruction.
This can reduce instructions and avoid register loads. Now that we generate
more SLTI/SLTIU instructions, absorb these into branches when it makes sense
to do so.

Removes more than 800 instructions from the Go binary on linux/riscv64.

Change-Id: I42c4e00486697acd4da7669d441b5690795f18ae
Reviewed-on: https://go-review.googlesource.com/c/go/+/428499
Reviewed-by: Wayne Zuo <[email protected]>
Run-TryBot: Joel Sing <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Joedian Reid <[email protected]>
Fold negation into subtraction and avoid double negation.

This removes around 500 instructions from the Go binary on riscv64.

Change-Id: I4aac6c87baa2a0759b180ba87876d488a23df6d7
Reviewed-on: https://go-review.googlesource.com/c/go/+/431105
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Joedian Reid <[email protected]>
Reviewed-by: Wayne Zuo <[email protected]>
Run-TryBot: Joel Sing <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
Delete some unused code, various cleanups to fix staticcheck warnings.

Change-Id: Ie475d57735a83351a4977f0dd4bc1387ce06a20e
Reviewed-on: https://go-review.googlesource.com/c/go/+/441935
Reviewed-by: David Chase <[email protected]>
As of CL 438347, multiple concurrents calls to Close should be safe.

This removes some indirection and may also make some programs that use
type-assertions marginally more efficient. For example, if a program
calls (*exec.Cmd).StdinPipe to obtain a pipe and then sets that as the
Stdout of another command, that program will now allow the second
command to inherit the file descriptor directly instead of copying
everything through a goroutine.

This will also cause calls to Close after the first to return an error
wrapping os.ErrClosed instead of nil. However, it seems unlikely that
programs will depend on that error behavior: if a program is calling
Write in a loop followed by Close, then if a racing Close occurs it is
likely that the Write would have already reported an error. (The only
programs likely to notice a change are those that call Close — without
Write! — after a call to Wait.)

Updates golang#56043.
Updates golang#9307.
Updates golang#6270.

Change-Id: Iec734b23acefcc7e7ad0c8bc720085bc45988efb
Reviewed-on: https://go-review.googlesource.com/c/go/+/439195
Reviewed-by: Ian Lance Taylor <[email protected]>
Auto-Submit: Bryan Mills <[email protected]>
Run-TryBot: Bryan Mills <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Change-Id: I0c8823b9c3c12f0f581b24db6a7aa5a0cd913224
Reviewed-on: https://go-review.googlesource.com/c/go/+/407537
Auto-Submit: Robert Griesemer <[email protected]>
Run-TryBot: Robert Griesemer <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>
go list -e -export puts errors running build actions on the load.Package
corresponding to the failed action rather than exiting with a non zero
exit code.

For golang#25842

Change-Id: I1fea85cc5a0557f514fe9d4ed3b6a858376fdcde
Reviewed-on: https://go-review.googlesource.com/c/go/+/437298
Run-TryBot: Bryan Mills <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Auto-Submit: Bryan Mills <[email protected]>
Reviewed-by: Michael Matloob <[email protected]>
Fixes golang#56156

Change-Id: Ib85ff45f0b0d0eac83c39606ee20b3a312e6e919
Reviewed-on: https://go-review.googlesource.com/c/go/+/442335
Run-TryBot: Ian Lance Taylor <[email protected]>
Auto-Submit: Ian Lance Taylor <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Run-TryBot: Ian Lance Taylor <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
…vers

When running go install --mod=readonly module@version. modfetch.GoSumFile
was not set, so the checksumOk check that's done when checking whether
we need to set the GoVersion from the go mod file was failing. Bypass
the checksumOk check when there's no GoSumFile.

For golang#54908

Change-Id: I56cf9d36a505b1223e6bf82a7d455746e2f09849
Reviewed-on: https://go-review.googlesource.com/c/go/+/439855
Reviewed-by: Bryan Mills <[email protected]>
Run-TryBot: Bryan Mills <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Michael Matloob <[email protected]>
This brings go/types error reporting closer to types2.

Except for removing the error functions and one manual correction,
these changes were made by regex-replacing:

check\.invalidAST\((.*), "      =>
check.errorf($1, InvalidSyntaxTree, invalidAST+"

check\.invalidOp\((.*), "       =>
check.errorf($1, invalidOp+"

check\.invalidArg\((.*), "      =>
check.errorf($1, invalidArg+"

A follow-up CL ensures that we use error instead of errorf where
possible.

Change-Id: Iac53dcd9c122b058f98d26d0fb307ef1dfe4e79b
Reviewed-on: https://go-review.googlesource.com/c/go/+/441955
Reviewed-by: Robert Griesemer <[email protected]>
Auto-Submit: Robert Griesemer <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
Run-TryBot: Robert Griesemer <[email protected]>
Checker.errorf calls now have an error code and thus require at
least 4 arguments.

Change-Id: Id01c30d5d3cc747ab0b3ba4001e88985192f2d80
Reviewed-on: https://go-review.googlesource.com/c/go/+/441957
Auto-Submit: Robert Griesemer <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>
Run-TryBot: Robert Griesemer <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
The errorcalls_test makes sure that we use error instead of errorf
where possible. Copied from types2 and adjusted for go/types.

Change-Id: Ib0572308c87e4415bf89aec8d64e662abe94754b
Reviewed-on: https://go-review.googlesource.com/c/go/+/441958
Reviewed-by: Robert Findley <[email protected]>
Run-TryBot: Robert Griesemer <[email protected]>
Auto-Submit: Robert Griesemer <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>
fakePC uses hash.Sum32, which returns an uint32. However, libfuzzer
trace/hook functions declare fakePC argument as int, causing overflow on
386 archs.

Fixing this by changing fakePC argument to uint to prevent the overflow.

Fixes golang#56141

Change-Id: I3994c461319983ab70065f90bf61539a363e0a2a
Reviewed-on: https://go-review.googlesource.com/c/go/+/441996
Auto-Submit: Cuong Manh Le <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
Run-TryBot: Cuong Manh Le <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>
Delete some unused code, and fix a few warnings from staticcheck.

Change-Id: I3d3a6f13dccffda060449948769c305d93a0389c
Reviewed-on: https://go-review.googlesource.com/c/go/+/441936
Reviewed-by: Bryan Mills <[email protected]>
Updates golang#38714

Change-Id: Ie5c7761ec003f84e649fa4c90be184a5ff6a0879
Reviewed-on: https://go-review.googlesource.com/c/go/+/419554
Reviewed-by: Bryan Mills <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Bryan Mills <[email protected]>
Reviewed-by: Than McIntosh <[email protected]>
Reviewed-by: Joedian Reid <[email protected]>
Auto-Submit: Bryan Mills <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
Skip TestTransportPersistConnLeakShortBody in HTTP/2 mode;
it's flaky and was previously HTTP/1-only.

Don't run TestTransportEventTrace and TestTransportIgnores408
in parallel.

Change-Id: I76bc540fac9317185ef7d414c9deafb35bc926b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/442495
Reviewed-by: Bryan Mills <[email protected]>
Run-TryBot: Damien Neil <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Initializes the R3 register with an available address in the stack. The addressed location is used to receive the number of bytes written by WriteFile.

Fixes golang#56080

Change-Id: I0368eb7a31d2d6a098fa9c26e074eb1114a92704
GitHub-Last-Rev: 23dbdb5
GitHub-Pull-Request: golang#56153
Reviewed-on: https://go-review.googlesource.com/c/go/+/442216
Reviewed-by: Cherry Mui <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Cherry Mui <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
Dragonfly and FreeBSD both used numerical values for these constants
chosen to be the same as on Solaris. For some reason, NetBSD did not,
and happens to interpret value 0 as P_ALL instead of P_PID
(see https://github.com/NetBSD/src/blob/3323ceb7822f98b3d2693aa26fd55c4ded6d8ba4/sys/sys/idtype.h#L43-L44).

Using the correct value for P_PID should cause wait6 to wait for the
correct process, which may help to avoid the deadlocks reported in

For golang#50138.
Updates golang#13987.

Change-Id: I0eacd1faee4a430d431fe48f9ccf837f49c42f39
Reviewed-on: https://go-review.googlesource.com/c/go/+/442478
Auto-Submit: Bryan Mills <[email protected]>
Reviewed-by: Benny Siegert <[email protected]>
Run-TryBot: Bryan Mills <[email protected]>
This change refactors span heap initialization. This change should just
be a no-op and just prepares for adding support for arenas.

For golang#51317.

Change-Id: Ie6f877ca10f86d26e7b6c4857b223589a351e253
Reviewed-on: https://go-review.googlesource.com/c/go/+/423364
Reviewed-by: Cherry Mui <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Michael Knyszek <[email protected]>
No-op change in preparation for arenas.

For golang#51317.

Change-Id: I0777f21763fcd34957b7e709580cf2b7a962ba67
Reviewed-on: https://go-review.googlesource.com/c/go/+/423365
Reviewed-by: Cherry Mui <[email protected]>
Run-TryBot: Michael Knyszek <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
This change makes (*mheap).sysAlloc take an explicit list of hints and a
boolean as to whether or not any newly-created heapArenas should be
registered in the full arena list.

This is a refactoring in preparation for arenas.

For golang#51317.

Change-Id: I0584a033fce3fcb60c5d0bc033d5fb8bd23b2378
Reviewed-on: https://go-review.googlesource.com/c/go/+/432078
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
Run-TryBot: Michael Knyszek <[email protected]>
This change adds an API to the runtime for arenas. A later CL can
potentially export it as an experimental API, but for now, just the
runtime implementation will suffice.

The purpose of arenas is to improve efficiency, primarily by allowing
for an application to manually free memory, thereby delaying garbage
collection. It comes with other potential performance benefits, such as
better locality, a better allocation strategy, and better handling of
interior pointers by the GC.

This implementation is based on one by [email protected] with a few
significant differences:
* The implementation lives entirely in the runtime (all layers).
* Arena chunks are the minimum of 8 MiB or the heap arena size. This
  choice is made because in practice 64 MiB appears to be way too large
  of an area for most real-world use-cases.
* Arena chunks are not unmapped, instead they're placed on an evacuation
  list and when there are no pointers left pointing into them, they're
  allowed to be reused.
* Reusing partially-used arena chunks no longer tries to find one used
  by the same P first; it just takes the first one available.
* In order to ensure worst-case fragmentation is never worse than 25%,
  only types and slice backing stores whose sizes are 1/4th the size of
  a chunk or less may be used. Previously larger sizes, up to the size
  of the chunk, were allowed.
* ASAN, MSAN, and the race detector are fully supported.
* Sets arena chunks to fault that were deferred at the end of mark
  termination (a non-public patch once did this; I don't see a reason
  not to continue that).

For golang#51317.

Change-Id: I83b1693a17302554cb36b6daa4e9249a81b1644f
Reviewed-on: https://go-review.googlesource.com/c/go/+/423359
Reviewed-by: Cherry Mui <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Michael Knyszek <[email protected]>
This change adds the arenas package and a function to reflect for
allocating from an arena via reflection, but all the new API is placed
behind a GOEXPERIMENT.

For golang#51317.

Change-Id: I026d46294e26ab386d74625108c19a0024fbcedc
Reviewed-on: https://go-review.googlesource.com/c/go/+/423361
Reviewed-by: Cherry Mui <[email protected]>
Run-TryBot: Michael Knyszek <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
While we're here, replace a couple uses of os.Environ with cmd.Environ.

For golang#51317.

Change-Id: Ic5cf4a887a7975a8281223eec0f94df230b6f095
Reviewed-on: https://go-review.googlesource.com/c/go/+/431955
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Run-TryBot: Michael Knyszek <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
Fixes golang#56150

Change-Id: Id990783562950ba8be7ce9526b7a811625f2190a
Reviewed-on: https://go-review.googlesource.com/c/go/+/442415
Run-TryBot: Ian Lance Taylor <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Reviewed-by: Michael Pratt <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Auto-Submit: Ian Lance Taylor <[email protected]>
Run-TryBot: Ian Lance Taylor <[email protected]>
Since we already provide the error code, the prefix can be deduced
automatically.

Except for the changes in errors.go, the updates were made with
regex find-and-replaces:

check\.error\((.+), InvalidSyntaxTree, invalidAST\+    =>
check.error($1, InvalidSyntaxTree,

check\.errorf\((.+), InvalidSyntaxTree, invalidAST\+    =>
check.errorf($1, InvalidSyntaxTree,

Change-Id: Ia02fc56ac7a8524bdf0c404ff2696435408327e9
Reviewed-on: https://go-review.googlesource.com/c/go/+/441975
Run-TryBot: Robert Griesemer <[email protected]>
Auto-Submit: Robert Griesemer <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
…missing method

Fixes golang#51025.

Change-Id: I469a705e7da059e7ac0b12b05beb9ed5d3617396
Reviewed-on: https://go-review.googlesource.com/c/go/+/438856
Reviewed-by: Robert Griesemer <[email protected]>
Auto-Submit: Robert Griesemer <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Robert Griesemer <[email protected]>
No test case because the problem can only happen for invalid data. Let
the fuzzer find cases like this.

Fixes golang#56129

Change-Id: I6c81933781384c5e2c8ba0fd99cec50455b9664a
Reviewed-on: https://go-review.googlesource.com/c/go/+/441976
Reviewed-by: Ian Lance Taylor <[email protected]>
Run-TryBot: Ian Lance Taylor <[email protected]>
Reviewed-by: Than McIntosh <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Joedian Reid <[email protected]>
Auto-Submit: Ian Lance Taylor <[email protected]>
Run-TryBot: Ian Lance Taylor <[email protected]>
Using importcfg instead of depending on the existence of .a files for
standard library packages will enable us to remove the .a files in a
future cl.

Change-Id: I6108384224508bc37d82fd990fc4a8649222502c
Reviewed-on: https://go-review.googlesource.com/c/go/+/440222
Reviewed-by: Bryan Mills <[email protected]>
Reviewed-by: Michael Matloob <[email protected]>
Run-TryBot: Michael Matloob <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
dr2chase and others added 27 commits November 3, 2022 18:46
Randomized feature enable/disable might be something we use to
help users debug any problems with changed loop variable capture,
and there's another CL that would like to use it to help in
locating places where "fused" multiply add instructions change
program behavior.

This CL:
- adds the ability to include an integer parameter (e.g. line number)
- replumbed the environment variable into a flag to simplify go build cache management
- but added an environment variable to allow flag setting through the environment
- which adds the possibility of switching on a different variable
  (if there's one built-in for variable capture, it shouldn't be GOSSAHASH)
- cleaned up the checking code
- adds tests for all the intended behavior
- removes the case for GSHS_LOGFILE; TBD whether we'll need to put that back
  or if there is another way.

Change-Id: I8503e1bb3dbc4a743aea696e04411ea7ab884787
Reviewed-on: https://go-review.googlesource.com/c/go/+/443063
Reviewed-by: Keith Randall <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Russ Cox <[email protected]>
Run-TryBot: David Chase <[email protected]>
For golang#56041

Change-Id: I6c98458b5c0d3b3636a53ee04fc97221f3fd8bbc
Reviewed-on: https://go-review.googlesource.com/c/go/+/444817
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Run-TryBot: Ian Lance Taylor <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Auto-Submit: Ian Lance Taylor <[email protected]>
Run-TryBot: xie cui <[email protected]>
Change-Id: I30783aa6a13ad8348fa24b27672d542a868f96de
GitHub-Last-Rev: c4584ad
GitHub-Pull-Request: golang#56526
Reviewed-on: https://go-review.googlesource.com/c/go/+/447217
Auto-Submit: Robert Griesemer <[email protected]>
Run-TryBot: Robert Griesemer <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
net.IP states that a 16-byte slice can still be an IPv4 address.
But after netip.Addr is introduced, it requires extra care to keep
it as an IPv4 address when converting it to a netip.Addr using
netip.AddrFromSlice.

To address this issue, let's change the cgo resolver to return
4-byte net.IP for IPv4. The change will save us 12 bytes too.

Please note that the go resolver already return IPv4 as 4-byte
slice.

The test TestResolverLookupIP has been modified to cover this
behavior. So no new test is added.

Fixes golang#53554.

Change-Id: I0dc2a59ad785c0c67a7bc22433105529f055997f
GitHub-Last-Rev: bd7bb2f
GitHub-Pull-Request: golang#53638
Reviewed-on: https://go-review.googlesource.com/c/go/+/415580
Auto-Submit: Damien Neil <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
Run-TryBot: Ian Lance Taylor <[email protected]>
We were mishandling {{range $i = .}}, treating it as though it were
{{range $i := .}}. That happened to work if $i were the most recently
declared variable, but not otherwise.

Fixes golang#56490

Change-Id: I222a009d671d86c06a980a54388e05f12101c00b
Reviewed-on: https://go-review.googlesource.com/c/go/+/446795
Run-TryBot: Ian Lance Taylor <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Rob Pike <[email protected]>
Run-TryBot: Ian Lance Taylor <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
Auto-Submit: Ian Lance Taylor <[email protected]>
We don't have a formatter for these files, so check here that
they are in the right form to allow 'cat next/*.txt >go1.X.txt'
at the end of each cycle.

Fix the api files that the check finds.

Change-Id: I0c5e4ab11751c7d0afce32503131d487313f41c0
Reviewed-on: https://go-review.googlesource.com/c/go/+/431335
Reviewed-by: Dmitri Shuralyov <[email protected]>
Auto-Submit: Russ Cox <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
Run-TryBot: Russ Cox <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
CC-BY-3.0 was shiny and new back in 2009, but CC-BY-4.0 is
generally preferred now. Update our CC-BY uses to CC-BY-4.0.

Google lawyers signed off on the overall CC-BY-4.0 update
and Renee French signed off on the update of the gopher license.

See also CL 447156.

Change-Id: I3908910d6011ed733271e595f761c773351b30f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/447275
TryBot-Result: Gopher Robot <[email protected]>
Auto-Submit: Russ Cox <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Run-TryBot: Russ Cox <[email protected]>
Currently, with PGO, the inliner uses node weights to decide if a
function is inlineable (with a larger budget). But the actual
inlining is determined by the weight of the call edge. There is a
discrepancy that, if a callee node is hot but the call edge is not,
it would not inlined, and marking the callee inlineable would of
no use.

Instead of using two kinds of weights, we just use the edge
weights to decide inlineability. If a function is the callee of a
hot call edge, its inlineability is determined with a larger
threshold. For a function that exceeds the regular inlining budget,
it is still inlined only when the call edge is hot, as it would
exceed the regular inlining cost for non-hot call sites, even if
it is marked inlineable.

For golang#55022.

Change-Id: I93fa9919fc6bcbb394e6cfe54ec96a96eede08f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/447015
Run-TryBot: Cherry Mui <[email protected]>
Reviewed-by: Michael Pratt <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
The -C flag is like tar -C or make -C: it changes to the named directory
early in command startup, before anything else happens.

Fixes golang#50332.

Change-Id: I8e4546f69044cb3a028d4d26dfba482b08cb845d
Reviewed-on: https://go-review.googlesource.com/c/go/+/421436
Reviewed-by: Bryan Mills <[email protected]>
Auto-Submit: Russ Cox <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Russ Cox <[email protected]>
In https://build.golang.org/log/d2eb315305bf3d513c490e7f85d56e9a016aacd2,
we observe a failure in TestPipeLookPathLeak due to an additional
descriptor (7) that was open at the start of the test being closed while
the test executes.

I haven't dug much into the failure, but it seems plausible to me that the
descriptor may have been opened by libc for some reason, and may have been
closed due to some sort of idle timeout or the completion of a background
initialization routine.

Since the test is looking for a leak, and closing an existing descriptor
does not indicate a leak, let's not fail the test if an existing descriptor
is unexpectedly closed.

Updates golang#5071.

Change-Id: I03973ddff6592c454cfcc790d6e56accd051dd52
Reviewed-on: https://go-review.googlesource.com/c/go/+/447235
Reviewed-by: Ian Lance Taylor <[email protected]>
Run-TryBot: Bryan Mills <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Two edge cases that were mentioned in the docs are actually impossible:

  * For NIST curves, ECDH can't fail, since the zero scalar is rejected
    by NewPrivateKey, the identity point is rejected by NewPublicKey,
    and NIST curves are a prime-order group.

    Let's call the inputs to scalar multiplication k and P, and the
    order of the group q. If k[P] is the identity, and also q[P] is the
    identity by definition, then P's order is a divisor of q-k, because

        k[P] + [q-k]P = q[P] = I

    P's order is either 1 or q, and can only be a divisor of q-k if it's
    1 (so P is the identity), or if k is zero.

  * For X25519, PrivateKey.PublicKey can't return the all-zero value,
    since no value is equivalent to zero after clamping.

    Clamping unsets the lowest three bit, sets the second-to-highest
    bit, and unsets the top bit; this means that a scalar equivalent to
    zero needs to be a multiple of 8*q, and needs to be between 2**254
    and 2**255-1, but 8*p > 2**255-1.

Tests for other exotic edge cases such as non-canonical point encodings,
clamping, points on the twist, and low-order components are covered by
x/crypto/wycheproof.

Change-Id: I731a878c58bd59aee5636211dc0f19ad8cfae9db
Reviewed-on: https://go-review.googlesource.com/c/go/+/425463
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
Run-TryBot: Filippo Valsorda <[email protected]>
Auto-Submit: Filippo Valsorda <[email protected]>
Reviewed-by: Russ Cox <[email protected]>
Change-Id: Ida7e756f01a2c115ac58bf10aa13b2f8fd57b6a1
GitHub-Last-Rev: 4694d39
GitHub-Pull-Request: golang#56537
Reviewed-on: https://go-review.googlesource.com/c/go/+/447436
Reviewed-by: Ian Lance Taylor <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
Run-TryBot: Ian Lance Taylor <[email protected]>
Auto-Submit: Ian Lance Taylor <[email protected]>
Per discussion with Roland Shoemaker, this updates
x509.CreateRevocationList to mirror the behavior of
x509.CreateCertificate, creating an internal struct for the ASN.1
encoding of the CRL. This allows us to switch the Issuer field type to
asn1.RawValue, bypassing the round-tripping issues of pkix.Name in most
scenarios.

Per linked ticket, this resolves issues where a non-Go created
certificate can be used to create CRLs which aren't correctly attested
to that certificate. In rare cases where the CRL issuer is validated
against the certificate's issuer (such as the linked JDK example), this
can result in failing to check this CRL for the candidate certificate.

Fixes golang#53754

Change-Id: If0adc053c081d6fb0b1ce47324c877eb2429a51f
GitHub-Last-Rev: 033115d
GitHub-Pull-Request: golang#53985
Reviewed-on: https://go-review.googlesource.com/c/go/+/418834
Reviewed-by: Roland Shoemaker <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
Run-TryBot: Roland Shoemaker <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Auto-Submit: Roland Shoemaker <[email protected]>
Remove the useless issueSpill and continue directly.

Change-Id: I085e566be6f7200235e1bfe1f56a8e959316386a
GitHub-Last-Rev: 84db90c
GitHub-Pull-Request: golang#56520
Reviewed-on: https://go-review.googlesource.com/c/go/+/447195
Run-TryBot: Keith Randall <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
Auto-Submit: Keith Randall <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
…tions

This is an oversight from https://go-review.googlesource.com/c/go/+/419875,
where script commands were refactored and factored out to a new package.

For golang#27494.

Change-Id: Ie606cab39f60859ee1da5165dcc94c8470c94325
Reviewed-on: https://go-review.googlesource.com/c/go/+/447575
Run-TryBot: Quim Muntal <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Auto-Submit: Bryan Mills <[email protected]>
Reviewed-by: Than McIntosh <[email protected]>
Currently, FromDir identifies a VCS checkout directory just by checking
whether it contains a specified file. This is not enough. For example,
although there is a ".git" file (a plain file, not a directory) in a
git submodule directory, this directory is not a git repository root.

This change takes the file mode into account. As of now, the filename
and file mode for the supported VCS tools are:

- Mercurial:    .hg             directory
- Git:          .git            directory
- Bazaar:       .bzr            directory
- Subversion:   .svn            directory
- Fossil:       .fslckout       plain file
- Fossil:       _FOSSIL_        plain file

This CL effectively reverts CL 30948 for golang#10322.

Fixes golang#53640.

Change-Id: Iea316c7e983232903bddb7e7f6dbaa55e8498685
GitHub-Last-Rev: 7a2d6ff
GitHub-Pull-Request: golang#56296
Reviewed-on: https://go-review.googlesource.com/c/go/+/443597
Auto-Submit: Bryan Mills <[email protected]>
Run-TryBot: Bryan Mills <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Than McIntosh <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
`TestScripts/svn` test suite fails if the host does not have a TZ
database installed.

This CL updates those tests so SVN formats dates using UTC, which
don't require a TZ database.

Fixes golang#56527

Change-Id: I20f3c03c3cedd7d748f4623dddc66bd04d1df318
Reviewed-on: https://go-review.googlesource.com/c/go/+/447335
Reviewed-by: Bryan Mills <[email protected]>
Run-TryBot: Quim Muntal <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Than McIntosh <[email protected]>
Auto-Submit: Bryan Mills <[email protected]>
Adding PCALIGN to addMulVVW assembler implementation
provides the following improvement on power10:

    AddMulVVW/1         3.36ns ± 0%    3.37ns ± 0%   +0.20%
    AddMulVVW/2         4.45ns ± 0%    4.44ns ± 0%   -0.25%
    AddMulVVW/3         5.44ns ± 0%    5.49ns ± 0%   +0.84%
    AddMulVVW/4         6.43ns ± 0%    6.34ns ± 0%   -1.33%
    AddMulVVW/5         7.87ns ± 0%    7.73ns ± 0%   -1.70%
    AddMulVVW/10        13.4ns ± 3%    12.4ns ± 7%   -7.07%
    AddMulVVW/100        112ns ± 0%     102ns ± 0%   -9.34%
    AddMulVVW/1000      1.09µs ± 0%    0.95µs ± 0%  -13.15%
    AddMulVVW/10000     10.9µs ± 0%     9.6µs ± 0%  -12.46%
    AddMulVVW/100000     109µs ± 0%      95µs ± 0%  -12.58%

Change-Id: Ic33d4f125c84d568f63e17cf99dc4df5ca9328d9
Reviewed-on: https://go-review.googlesource.com/c/go/+/447236
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
Reviewed-by: Russ Cox <[email protected]>
Run-TryBot: Lynn Boger <[email protected]>
Reviewed-by: Paul Murphy <[email protected]>
Reviewed-by: Archana Ravindar <[email protected]>
Currently in PGO we use a percentage threshold to determine if a
callsite is hot. This CL uses a different method -- treating the
hottest callsites that make up cumulatively top X% of total edge
weights as hot (X=95 for now). This default might work better for
a wider range of profiles. (The absolute threshold can still be
changed by a flag.)

For golang#55022.

Change-Id: I7e3b6f0c3cf23f9a89dd5994c10075b498bf14ee
Reviewed-on: https://go-review.googlesource.com/c/go/+/447016
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Cherry Mui <[email protected]>
Reviewed-by: Michael Pratt <[email protected]>
…l linking

On some platforms, building PIE needs external linking, which
cannot run if cgo is not available.

Change-Id: I6d504aed0f0442cda0355d0beac606ad365e2046
Reviewed-on: https://go-review.googlesource.com/c/go/+/447616
Reviewed-by: Than McIntosh <[email protected]>
Run-TryBot: Cherry Mui <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
This adds a -d debug flag "fmahash" for hashcode search for
floating point architecture-dependent problems. This variable has no
effect on architectures w/o fused-multiply-add.

This was rebased onto the GOSSAHASH renovation so that this could have
its own dedicated environment variable, and so that it would be
cheap (a nil check) to check it in the normal case.

Includes a basic test of the trigger plumbing.

Sample use (on arm64, ppc64le, s390x):

% GOCOMPILEDEBUG=fmahash=001110110 \
  go build -o foo cmd/compile/internal/ssa/testdata/fma.go
fmahash triggered main.main:24 101111101101111001110110
GOFMAHASH triggered main.main:20 010111010000101110111011
1.0000000000000002 1.0000000000000004 -2.220446049250313e-16
exit status 1

The intended use is in conjunction with github.com/dr2chase/gossahash,
which will probably acquire a flag "-fma" to streamline its use. This
tool+use was inspired by an ad hoc use of this technique "in anger"
to debug this very problem.  This is also a dry-run for using this
same technique to identify code sensitive to loop variable
lifetime/capture, should we make that change.

Example intended use, with current search tool (using old environment
variable), for a test example:

gossahash -e GOFMAHASH GOMAGIC=GOFMAHASH go run fma.go
Trying go args=[...], env=[GOFMAHASH=1 GOMAGIC=GOFMAHASH]
go failed (81 distinct triggers): exit status 1
Trying go args=[...], env=[GOFMAHASH=11 GOMAGIC=GOFMAHASH]
go failed (39 distinct triggers): exit status 1
Trying go args=[...], env=[GOFMAHASH=011 GOMAGIC=GOFMAHASH]
go failed (18 distinct triggers): exit status 1
Trying go args=[...], env=[GOFMAHASH=0011 GOMAGIC=GOFMAHASH]
Trying go args=[...], env=[GOFMAHASH=1011 GOMAGIC=GOFMAHASH]
...
Trying go args=[...], env=[GOFMAHASH=0110111011 GOMAGIC=GOFMAHASH]
Trying go args=[...], env=[GOFMAHASH=1110111011 GOMAGIC=GOFMAHASH]
go failed (2 distinct triggers): exit status 1
Trigger string is 'GOFMAHASH triggered math.qzero:427 111111101010011110111011', repeated 6 times
Trigger string is 'GOFMAHASH triggered main.main:20 010111010000101110111011', repeated 1 times
Trying go args=[...], env=[GOFMAHASH=01110111011 GOMAGIC=GOFMAHASH]
go failed (1 distinct triggers): exit status 1
Trigger string is 'GOFMAHASH triggered main.main:20 010111010000101110111011', repeated 1 times
Review GSHS_LAST_FAIL.0.log for failing run
FINISHED, suggest this command line for debugging:
GOSSAFUNC='main.main:20 010111010000101110111011' \
GOFMAHASH=01110111011 GOMAGIC=GOFMAHASH go run fma.go

Change-Id: Ifa22dd8f1c37c18fc8a4f7c396345a364bc367d5
Reviewed-on: https://go-review.googlesource.com/c/go/+/394754
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: David Chase <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>
In following with Roland's TODO, switch TestDisableSHA1ForCertOnly to ParseRevocationList(...) over ParseCRL(...).

Change-Id: I8cdaf04ad0a1c8b94303415ae41933657067041e
GitHub-Last-Rev: bb2ef76
GitHub-Pull-Request: golang#56541
Reviewed-on: https://go-review.googlesource.com/c/go/+/447036
Reviewed-by: Roland Shoemaker <[email protected]>
Run-TryBot: Roland Shoemaker <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>
Auto-Submit: Roland Shoemaker <[email protected]>
The stack pointer must lie within system stack limits
when Control Flow Guard (CFG) is enabled on Windows.

This CL updates runtime.sigtramp to honor this restriction by
porting some code from the windows/arm64 version, which
already supports CFG.

Fixes golang#53560

Change-Id: I7f88f9ae788b2bac38aac898b2567f1bea62f8f3
Reviewed-on: https://go-review.googlesource.com/c/go/+/437559
Reviewed-by: Matthew Dempsky <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Michael Pratt <[email protected]>
Run-TryBot: Michael Pratt <[email protected]>
Fixes golang#56542

Change-Id: I294856f8fb4d49393310ec92ab40fb7d841b6570
GitHub-Last-Rev: a456340
GitHub-Pull-Request: golang#56545
Reviewed-on: https://go-review.googlesource.com/c/go/+/447198
Reviewed-by: Bryan Mills <[email protected]>
Run-TryBot: Bryan Mills <[email protected]>
Auto-Submit: Bryan Mills <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
…ing heap growth

Benchmarking suggests about a 14-17% reduction in user build time,
about 3.5-7.8% reduction for wall time.  This helps most builds
because small packages are common. Latest benchmarks (after the last
round of improvement):

(12 processors) https://perf.golang.org/search?q=upload:20221102.20
(GOMAXPROCS=2)  https://perf.golang.org/search?q=upload:20221103.1
(48 processors) https://perf.golang.org/search?q=upload:20221102.19

(The number of compiler workers is capped at min(4, GOMAXPROCS))

An earlier, similar version of this CL at one point observed a 27%
reduction in user build time (building 40+ benchmarks, 20 times), but
the current form is judged to be the most reliable; it may be
profitable to tweak the numbers slightly later, and/or to adjust the
number of compiler workers.

We've talked about doing this in the past, the "new"(ish) metrics
package makes it a more tractable proposition.

The method here is:

1. If os.Getenv("GOGC") is empty, then increase GOGC to a large value,
calculated to grow the heap to 32 + 4 * compile_parallelism before a
GC occurs (e.g., on a >= 4 processor box, 64M).  In practice,
sometimes GC occurs before that, but this still results in fewer GCs
and saved time.  This is "heap goal".

2. Use a finalizer to approximately detect when GC occurs, and use
runtime metrics to track progress towards the goal heap size,
readjusting GOGC to retarget it as necessary.  Reset GOGC to 100 when
the heap is "close enough" to the goal.

One feared failure mode of doing this is that the finalizer will be
slow to run and the heap will grow exceptionally large before GOGC is
reset; I monitored the heap size at reset and exit across several
boxes with a variety of processor counts and extra noise
(including several builds in parallel, including a laptop with a busy
many-tabs browser running) and overshoot effectively does not occur.

In some cases the compiler's heap grows so rapidly that estimated live
exceeds the GC goal, but this is not delayed-finalizer overshoot; the
compiler is just using that much memory.  In a small number of cases
(3% of GCs in make.bash) the new goal is larger than predicted by as
much as 38%, so check for that and redo the adjustment.

I considered instead using the maximum heap size limit +
GC-detecting-finalizer + reset instead, but to me that seemed like it
might have a worse bad-case outcome; if the reset is delayed, it's
possible the GC would start running frequently, making it harder to
run the finalizer, reach 50% utilization, and the extra GCs would
lose the advantage.  This might also perform badly in the case that a
rapidly growing heap outruns goal.  In practice, this sort of
overshoot hasn't been observed, and a goal of 64M is small enough to
tolerate plenty of overshoot anyway.

This version of the CL includes a comment urging anyone who sees the
code and thinks it would work for them, to update a bug (to be
created if the CL is approved) with information about their
situation/experience, so that we may consider creating some more
official and reliable way of obtaining the same result.

Change-Id: I45df1c927c1a7d7503ade1abd1a3300e27516633
Reviewed-on: https://go-review.googlesource.com/c/go/+/436235
Run-TryBot: David Chase <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
@romaindoumenc
Copy link
Author

Will re-submit with single version, integrating existing comments.

@romaindoumenc romaindoumenc deleted the master branch November 3, 2022 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.