Prologue

In my previous post, I talked about my fix for a long time go comamnd bug. Then I bet you that the fix is unlikely to cause any issue.

I was wrong, and here’s the story.

The bug

One day in Gopher Slack #tools, dominikhonnef (an experienced/respectful Gopher) asked:

when testing compiler changes/bisecting compiler bugs, is it enough to do go install cmd/compile? till now I’ve always done a full make.bash, but that’s a bit slow

We traded some sentences, and though I showed him another way instead of using make.bash, I didn’t pay much attention to that, since when make.bash in my M1 is pretty fast.

The day after, @bcmills (owner of cmd/go) said:

@dominikh, alternately, we could make make.bash properly saturate big machines… 😅

Then @dominikhonnef showed:

go 1.17: 203.46user 16.89system 0:35.27elapsed 624%CPU (0avgtext+0avgdata 863856maxresident)k
master: 204.67user 15.30system 0:56.85elapsed 386%CPU (0avgtext+0avgdata 833420maxresident)k

So make.bash becomes slower between go 1.17 and tip, there must be a regression introduced during go 1.18 development cycle. More surprising, @dominikhonnef bisected to this commit:

commit a6ff433d6a927e8ad8eaa6828127233296d12ce5 (HEAD, refs/bisect/bad)
Author: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Date:   Wed Aug 25 14:56:01 2021 +0700

    cmd/go: pass -gcflags after other flags generated by the go command
    
    Otherwise, any gc flags set by user using "-gcflags" won't have effect.
    
    Fixes #47682
    
    Change-Id: Icd365577cba1f64f6c7b8320d0c9019de9f062f6
    Reviewed-on: https://go-review.googlesource.com/c/go/+/344909
    Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
    Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
    TryBot-Result: Go Bot <gobot@golang.org>
    Reviewed-by: Jay Conrod <jayconrod@google.com>

which is authored by me. @dominikhonnef even went further and show that -c flag isn’t passed to the compiler anymore:

well, if looking at random output by go build -x is anything to go by… there is no more -c being passed to go tool compile

I wonder why just swapping the order of flags causes a flag disappear?

Re-reading CL 344909, I realize that it did more things than just swapping the flags!

If you read Fixing section in my previous post, you will see not only the two flags was swapped, but also the misleading flags gcargs was removed, and it was merged in to gcflags variable. However, that variable is used by function gcBackendConcurrency to determine the backend compiler concurrency level for a package compilation. Here’s part of that function:

CheckFlags:
	for _, flag := range gcflags {
		// Concurrent compilation is presumed incompatible with any gcflags,
		// except for known commonly used flags.
		// If the user knows better, they can manually add their own -c to the gcflags.
		switch flag {
		case "-N", "-l", "-S", "-B", "-C", "-I":
			// OK
		default:
			canDashC = false
			break CheckFlags
		}
	}

Since when the gcflags now contains flags from gcargs, so more flags are present in gcflags than before, canDashC will be false.

The fix

I sent CL 351049 to fix the bug. It’s quite simple, just don’t merge those flags set like they were before, and we’re good.

Note

When writing test case for the bug, I found an issue that go command does not do concurrent compilation on darwin/arm64!

That’s the reason I don’t see the slow down when running make.bash in my M1 after CL 344909.

Updated 24/09/2021

I sent a followup CL 351849, hopefully making the code easier to understand/maintain and prevent subtle bug like this.

Epilogue

A lesson that I learn after this story, never do refactoring+fixing in the same CL (or commit). Fix the bug with one CL and do refactoring in the followup one.

Thanks for reading so far!

Till next time!