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

cmd/go: '-gcflags=all=-l' flag does not disable inlining for all packages #27708

Closed
houxl123 opened this issue Sep 17, 2018 · 10 comments

Comments

Projects
None yet
3 participants
@houxl123
Copy link

commented Sep 17, 2018

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.11 linux/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

ubuntu 18.04

What did you do?

'go test -gcflags=all=-l' my project, but failed because disable inlining is invalid on http server.
The following program can be reproduced.
https://github.com/houxl123/test_inline/

What did you expect to see?

go test success in go1.11

What did you see instead?

go test fail

@bcmills

This comment has been minimized.

Copy link
Member

commented Sep 17, 2018

Just to set expectations: github.com/bouk/monkey makes assumptions about the structure of the compiled code that are not at all guaranteed by the language spec, so you should not expect it to continue to work in general.

If you're using it for tests that you actually care about, I recommend restructuring the code so that you can test it via its exported API (without relying on the implementation details of the compiler).

@bcmills bcmills changed the title '-gcflags=all=-l' disables inlining failure in `http server` cmd/compile: '-gcflags=all=-l' disables inlining in http.Server Sep 17, 2018

@bcmills

This comment was marked as outdated.

Copy link
Member

commented Sep 17, 2018

Here is the assembly code for the first part of the TestHttp function (up to the defer), generated on my machine using go build -gcflags=all=-l with go version devel +b2fcfc1a50 Wed Sep 12 07:50:10 2018 +0000 linux/amd64 (more recent that 1.11):

0000000000672230 <test_inline/test.TestHttp>:
  672230:       64 48 8b 0c 25 f8 ff    mov    %fs:0xfffffffffffffff8,%rcx
  672237:       ff ff
  672239:       48 8d 44 24 b8          lea    -0x48(%rsp),%rax
  67223e:       48 3b 41 10             cmp    0x10(%rcx),%rax
  672242:       0f 86 8e 03 00 00       jbe    6725d6 <test_inline/test.TestHttp+0x3a6>
  672248:       48 81 ec c8 00 00 00    sub    $0xc8,%rsp
  67224f:       48 89 ac 24 c0 00 00    mov    %rbp,0xc0(%rsp)
  672256:       00
  672257:       48 8d ac 24 c0 00 00    lea    0xc0(%rsp),%rbp
  67225e:       00
  67225f:       48 8d 05 1a 1f 04 00    lea    0x41f1a(%rip),%rax        # 6b4180 <type.*+0x40f60>
  672266:       48 89 04 24             mov    %rax,(%rsp)
  67226a:       48 8d 0d f7 a9 0b 00    lea    0xba9f7(%rip),%rcx        # 72cc68 <go.func.*+0xbc5>
  672271:       48 89 4c 24 08          mov    %rcx,0x8(%rsp)
  672276:       48 89 44 24 10          mov    %rax,0x10(%rsp)
  67227b:       48 8d 05 de a9 0b 00    lea    0xba9de(%rip),%rax        # 72cc60 <go.func.*+0xbbd>
  672282:       48 89 44 24 18          mov    %rax,0x18(%rsp)
  672287:       e8 a4 f1 ff ff          callq  671430 <github.com/bouk/monkey.Patch>
  67228c:       48 8d 05 2d 61 0e 00    lea    0xe612d(%rip),%rax        # 7583c0 <go.itab.net/http.HandlerFunc,net/http.Handler>
  672293:       48 89 04 24             mov    %rax,(%rsp)
  672297:       48 8d 05 d2 a9 0b 00    lea    0xba9d2(%rip),%rax        # 72cc70 <go.func.*+0xbcd>
  67229e:       48 89 44 24 08          mov    %rax,0x8(%rsp)
  6722a3:       e8 38 dd ff ff          callq  66ffe0 <net/http/httptest.NewServer>
  6722a8:       48 8b 44 24 10          mov    0x10(%rsp),%rax
  6722ad:       48 89 44 24 48          mov    %rax,0x48(%rsp)
  6722b2:       c7 04 24 08 00 00 00    movl   $0x8,(%rsp)
  6722b9:       48 8d 0d 10 a4 0b 00    lea    0xba410(%rip),%rcx        # 72c6d0 <go.func.*+0x62d>
  6722c0:       48 89 4c 24 08          mov    %rcx,0x8(%rsp)
  6722c5:       e8 96 e6 db ff          callq  430960 <runtime.deferproc>

Note the instruction lea 0xba9d2(%rip),%rax at offset 672297.

That instruction uses PC-relative addressing to load the address of the relocation entry for the function handle.SayMax.
If you look at the contents of the loaded address at that point, you'll find that it contains a pointer to the test_inline/handle.SayMax function (0x671f90 in this build), which is not inlined.

I don't know why the call to monkey.Patch doesn't replace the code as you expect it to, but the -l flag seems to be working as documented here.

@bcmills bcmills closed this Sep 17, 2018

@bcmills bcmills changed the title cmd/compile: '-gcflags=all=-l' disables inlining in http.Server cmd/compile: 'github.com/bouk/monkey.Patch' can no longer replace argument to 'httptest.NewServer' Sep 17, 2018

@bcmills

This comment was marked as outdated.

Copy link
Member

commented Sep 17, 2018

If you expect monkey.Patch to work with Go 1.11, I suggest that you file an issue against github.com/bouk/monkey. The problem here does not seem to be in the Go compiler.

@bcmills

This comment has been minimized.

Copy link
Member

commented Sep 17, 2018

(CC @bouk)

@bcmills

This comment has been minimized.

Copy link
Member

commented Sep 17, 2018

I stand corrected: it does appear that handle.Max is being inlined — or at least constant-folded — into handle.SayMax.

0000000000671f90 <test_inline/handle.SayMax>:
  671f90:       64 48 8b 0c 25 f8 ff    mov    %fs:0xfffffffffffffff8,%rcx
  671f97:       ff ff
  671f99:       48 3b 61 10             cmp    0x10(%rcx),%rsp
  671f9d:       0f 86 ba 00 00 00       jbe    67205d <test_inline/handle.SayMax+0xcd>
  671fa3:       48 83 ec 68             sub    $0x68,%rsp
  671fa7:       48 89 6c 24 60          mov    %rbp,0x60(%rsp)
  671fac:       48 8d 6c 24 60          lea    0x60(%rsp),%rbp
  671fb1:       0f 57 c0                xorps  %xmm0,%xmm0
  671fb4:       0f 11 44 24 50          movups %xmm0,0x50(%rsp)
  671fb9:       48 8d 05 c0 22 03 00    lea    0x322c0(%rip),%rax        # 6a4280 <type.*+0x31060>
  671fc0:       48 89 04 24             mov    %rax,(%rsp)
  671fc4:       48 c7 44 24 08 02 00    movq   $0x2,0x8(%rsp)
  671fcb:       00 00
  671fcd:       e8 3e 9b d9 ff          callq  40bb10 <runtime.convT2E64>
  671fd2:       48 8b 44 24 10          mov    0x10(%rsp),%rax
  671fd7:       48 8b 4c 24 18          mov    0x18(%rsp),%rcx
  671fdc:       48 89 44 24 50          mov    %rax,0x50(%rsp)
  671fe1:       48 89 4c 24 58          mov    %rcx,0x58(%rsp)
  671fe6:       48 8d 05 93 05 05 00    lea    0x50593(%rip),%rax        # 6c2580 <type.*+0x4f360>
  671fed:       48 89 04 24             mov    %rax,(%rsp)
  671ff1:       48 8b 44 24 70          mov    0x70(%rsp),%rax
  671ff6:       48 89 44 24 08          mov    %rax,0x8(%rsp)
  671ffb:       48 8b 44 24 78          mov    0x78(%rsp),%rax
  672000:       48 89 44 24 10          mov    %rax,0x10(%rsp)
  672005:       e8 f6 a0 d9 ff          callq  40c100 <runtime.convI2I>
  67200a:       48 8b 44 24 18          mov    0x18(%rsp),%rax
  67200f:       48 8b 4c 24 20          mov    0x20(%rsp),%rcx
  672014:       48 89 04 24             mov    %rax,(%rsp)
  672018:       48 89 4c 24 08          mov    %rcx,0x8(%rsp)
  67201d:       48 8d 05 fe 4b 0a 00    lea    0xa4bfe(%rip),%rax        # 716c22 <go.string.*+0x42>
  672024:       48 89 44 24 10          mov    %rax,0x10(%rsp)
  672029:       48 c7 44 24 18 02 00    movq   $0x2,0x18(%rsp)
  672030:       00 00
  672032:       48 8d 44 24 50          lea    0x50(%rsp),%rax
  672037:       48 89 44 24 20          mov    %rax,0x20(%rsp)
  67203c:       48 c7 44 24 28 01 00    movq   $0x1,0x28(%rsp)
  672043:       00 00
  672045:       48 c7 44 24 30 01 00    movq   $0x1,0x30(%rsp)
  67204c:       00 00
  67204e:       e8 cd 60 e4 ff          callq  4b8120 <fmt.Fprintf>
  672053:       48 8b 6c 24 60          mov    0x60(%rsp),%rbp
  672058:       48 83 c4 68             add    $0x68,%rsp
  67205c:       c3                      retq
  67205d:       e8 4e f6 de ff          callq  4616b0 <runtime.morestack_noctxt>
  672062:       e9 29 ff ff ff          jmpq   671f90 <test_inline/handle.SayMax>

@bcmills bcmills reopened this Sep 17, 2018

@bcmills bcmills changed the title cmd/compile: 'github.com/bouk/monkey.Patch' can no longer replace argument to 'httptest.NewServer' cmd/compile: '-l' flag does not disable inlining for function with constant result Sep 17, 2018

@bcmills

This comment has been minimized.

Copy link
Member

commented Sep 17, 2018

CC @randall77 @josharian @martisch

It's not obvious to me whether the -l flag is intended to apply to constant-folding, or only to inlining proper.

@bcmills bcmills added this to the Go1.12 milestone Sep 17, 2018

@cherrymui

This comment has been minimized.

Copy link
Contributor

commented Sep 17, 2018

I think if inlining is disabled, it should not do constant-folding through the call. In fact, adding //go:noinline to Max, I see an actual CALL to Max. And I think there shouldn't be any difference between -l and //go:noinline.

In fact, I tried

go test -c -a -x -work -gcflags=all=-l github.com/houxl123/test_inline/test

Reading the build log, -l is not applied to some of the compilation, including compiling handle.go.

compile -o $WORK/b108/_pkg_.a -trimpath $WORK/b108 -p test_inline/handle -complete -buildid CsFQHdxtcVc0reS8Hejv/CsFQHdxtcVc0reS8Hejv -D "" -importcfg $WORK/b108/importcfg -pack -c=4 ./handle.go

More complete list for all compiles:

compile -o $WORK/b004/_pkg_.a -trimpath $WORK/b004 -l -p errors
compile -o $WORK/b014/_pkg_.a -trimpath $WORK/b014 -l -p runtime/internal/sys
compile -o $WORK/b008/_pkg_.a -trimpath $WORK/b008 -l -p internal/race
compile -o $WORK/b012/_pkg_.a -trimpath $WORK/b012 -l -p internal/cpu
compile -o $WORK/b013/_pkg_.a -trimpath $WORK/b013 -l -p runtime/internal/atomic
compile -o $WORK/b015/_pkg_.a -trimpath $WORK/b015 -l -p sync/atomic
compile -o $WORK/b022/_pkg_.a -trimpath $WORK/b022 -l -p unicode
compile -o $WORK/b023/_pkg_.a -trimpath $WORK/b023 -l -p unicode/utf8
compile -o $WORK/b029/_pkg_.a -trimpath $WORK/b029 -l -p math/bits
compile -o $WORK/b051/_pkg_.a -trimpath $WORK/b051 -p container/list
compile -o $WORK/b011/_pkg_.a -trimpath $WORK/b011 -l -p internal/bytealg
compile -o $WORK/b019/_pkg_.a -trimpath $WORK/b019 -l -p internal/testlog
compile -o $WORK/b055/_pkg_.a -trimpath $WORK/b055 -p crypto/internal/subtle
compile -o $WORK/b056/_pkg_.a -trimpath $WORK/b056 -p crypto/subtle
compile -o $WORK/b026/_pkg_.a -trimpath $WORK/b026 -l -p math
compile -o $WORK/b080/_pkg_.a -trimpath $WORK/b080 -p vendor/golang_org/x/crypto/cryptobyte/asn1
compile -o $WORK/b082/_pkg_.a -trimpath $WORK/b082 -p vendor/golang_org/x/net/dns/dnsmessage
compile -o $WORK/b083/_pkg_.a -trimpath $WORK/b083 -p internal/nettrace
compile -o $WORK/b010/_pkg_.a -trimpath $WORK/b010 -l -p runtime
compile -o $WORK/b090/_pkg_.a -trimpath $WORK/b090 -p vendor/golang_org/x/crypto/curve25519
compile -o $WORK/b028/_pkg_.a -trimpath $WORK/b028 -l -p strconv
compile -o $WORK/b069/_pkg_.a -trimpath $WORK/b069 -p crypto/rc4
compile -o $WORK/b085/_pkg_.a -trimpath $WORK/b085 -p runtime/cgo
compile -o $WORK/b007/_pkg_.a -trimpath $WORK/b007 -l -p sync
compile -o $WORK/b006/_pkg_.a -trimpath $WORK/b006 -l -p io
compile -o $WORK/b084/_pkg_.a -trimpath $WORK/b084 -p internal/singleflight
compile -o $WORK/b058/_pkg_.a -trimpath $WORK/b058 -p math/rand
compile -o $WORK/b027/_pkg_.a -trimpath $WORK/b027 -l -p reflect
compile -o $WORK/b016/_pkg_.a -trimpath $WORK/b016 -l -p syscall
compile -o $WORK/b021/_pkg_.a -trimpath $WORK/b021 -p bytes
compile -o $WORK/b044/_pkg_.a -trimpath $WORK/b044 -p hash
compile -o $WORK/b047/_pkg_.a -trimpath $WORK/b047 -p text/tabwriter
compile -o $WORK/b031/_pkg_.a -trimpath $WORK/b031 -p strings
compile -o $WORK/b043/_pkg_.a -trimpath $WORK/b043 -p hash/crc32
compile -o $WORK/b060/_pkg_.a -trimpath $WORK/b060 -p crypto
compile -o $WORK/b036/_pkg_.a -trimpath $WORK/b036 -p bufio
compile -o $WORK/b064/_pkg_.a -trimpath $WORK/b064 -p crypto/internal/randutil
compile -o $WORK/b065/_pkg_.a -trimpath $WORK/b065 -p crypto/sha512
compile -o $WORK/b067/_pkg_.a -trimpath $WORK/b067 -p crypto/hmac
compile -o $WORK/b017/_pkg_.a -trimpath $WORK/b017 -l -p time
compile -o $WORK/b018/_pkg_.a -trimpath $WORK/b018 -l -p internal/syscall/unix
compile -o $WORK/b068/_pkg_.a -trimpath $WORK/b068 -p crypto/md5
compile -o $WORK/b071/_pkg_.a -trimpath $WORK/b071 -p crypto/sha1
compile -o $WORK/b072/_pkg_.a -trimpath $WORK/b072 -p crypto/sha256
compile -o $WORK/b094/_pkg_.a -trimpath $WORK/b094 -p vendor/golang_org/x/text/transform
compile -o $WORK/b106/_pkg_.a -trimpath $WORK/b106 -p path
compile -o $WORK/b030/_pkg_.a -trimpath $WORK/b030 -p sort
compile -o $WORK/b042/_pkg_.a -trimpath $WORK/b042 -p encoding/binary
compile -o $WORK/b005/_pkg_.a -trimpath $WORK/b005 -l -p internal/poll
compile -o $WORK/b038/_pkg_.a -trimpath $WORK/b038 -p regexp/syntax
compile -o $WORK/b078/_pkg_.a -trimpath $WORK/b078 -p encoding/base64
compile -o $WORK/b054/_pkg_.a -trimpath $WORK/b054 -p crypto/cipher
compile -o $WORK/b089/_pkg_.a -trimpath $WORK/b089 -p vendor/golang_org/x/crypto/poly1305
compile -o $WORK/b003/_pkg_.a -trimpath $WORK/b003 -l -p os
compile -o $WORK/b077/_pkg_.a -trimpath $WORK/b077 -p encoding/pem
compile -o $WORK/b061/_pkg_.a -trimpath $WORK/b061 -p crypto/des
compile -o $WORK/b053/_pkg_.a -trimpath $WORK/b053 -p crypto/aes
compile -o $WORK/b088/_pkg_.a -trimpath $WORK/b088 -p vendor/golang_org/x/crypto/internal/chacha20
compile -o $WORK/b025/_pkg_.a -trimpath $WORK/b025 -l -p fmt
compile -o $WORK/b046/_pkg_.a -trimpath $WORK/b046 -p path/filepath
compile -o $WORK/b032/_pkg_.a -trimpath $WORK/b032 -p runtime/debug
compile -o $WORK/b087/_pkg_.a -trimpath $WORK/b087 -p vendor/golang_org/x/crypto/chacha20poly1305
compile -o $WORK/b037/_pkg_.a -trimpath $WORK/b037 -p regexp
compile -o $WORK/b045/_pkg_.a -trimpath $WORK/b045 -p io/ioutil
compile -o $WORK/b034/_pkg_.a -trimpath $WORK/b034 -p context
compile -o $WORK/b049/_pkg_.a -trimpath $WORK/b049 -p bou.ke/monkey -complete
compile -o $WORK/b041/_pkg_.a -trimpath $WORK/b041 -p compress/flate
compile -o $WORK/b024/_pkg_.a -trimpath $WORK/b024 -p flag
compile -o $WORK/b057/_pkg_.a -trimpath $WORK/b057 -p math/big
compile -o $WORK/b076/_pkg_.a -trimpath $WORK/b076 -p encoding/hex
compile -o $WORK/b033/_pkg_.a -trimpath $WORK/b033 -p runtime/trace
compile -o $WORK/b086/_pkg_.a -trimpath $WORK/b086 -p net/url
compile -o $WORK/b096/_pkg_.a -trimpath $WORK/b096 -p log
compile -o $WORK/b020/_pkg_.a -trimpath $WORK/b020 -p testing
compile -o $WORK/b040/_pkg_.a -trimpath $WORK/b040 -p compress/gzip
compile -o $WORK/b095/_pkg_.a -trimpath $WORK/b095 -p vendor/golang_org/x/text/unicode/bidi
compile -o $WORK/b097/_pkg_.a -trimpath $WORK/b097 -p vendor/golang_org/x/text/unicode/norm
compile -o $WORK/b039/_pkg_.a -trimpath $WORK/b039 -p runtime/pprof
compile -o $WORK/b100/_pkg_.a -trimpath $WORK/b100 -p vendor/golang_org/x/net/http2/hpack
compile -o $WORK/b093/_pkg_.a -trimpath $WORK/b093 -p vendor/golang_org/x/text/secure/bidirule
compile -o $WORK/b101/_pkg_.a -trimpath $WORK/b101 -p mime
compile -o $WORK/b103/_pkg_.a -trimpath $WORK/b103 -p mime/quotedprintable
compile -o $WORK/b105/_pkg_.a -trimpath $WORK/b105 -p net/http/internal
compile -o $WORK/b052/_pkg_.a -trimpath $WORK/b052 -p crypto/rand
compile -o $WORK/b035/_pkg_.a -trimpath $WORK/b035 -p testing/internal/testdeps
compile -o $WORK/b063/_pkg_.a -trimpath $WORK/b063 -p crypto/elliptic
compile -o $WORK/b066/_pkg_.a -trimpath $WORK/b066 -p encoding/asn1
compile -o $WORK/b074/_pkg_.a -trimpath $WORK/b074 -p crypto/dsa
compile -o $WORK/b070/_pkg_.a -trimpath $WORK/b070 -p crypto/rsa
compile -o $WORK/b092/_pkg_.a -trimpath $WORK/b092 -p vendor/golang_org/x/net/idna
compile -o $WORK/b062/_pkg_.a -trimpath $WORK/b062 -p crypto/ecdsa
compile -o $WORK/b075/_pkg_.a -trimpath $WORK/b075 -p crypto/x509/pkix
compile -o $WORK/b079/_pkg_.a -trimpath $WORK/b079 -p vendor/golang_org/x/crypto/cryptobyte
compile -o $WORK/b081/_pkg_.a -trimpath $WORK/b081 -p net
compile -o $WORK/b099/_pkg_.a -trimpath $WORK/b099 -p vendor/golang_org/x/net/http/httpproxy
compile -o $WORK/b098/_pkg_.a -trimpath $WORK/b098 -p net/textproto
compile -o $WORK/b073/_pkg_.a -trimpath $WORK/b073 -p crypto/x509
compile -o $WORK/b091/_pkg_.a -trimpath $WORK/b091 -p vendor/golang_org/x/net/http/httpguts
compile -o $WORK/b102/_pkg_.a -trimpath $WORK/b102 -p mime/multipart
compile -o $WORK/b059/_pkg_.a -trimpath $WORK/b059 -p crypto/tls
compile -o $WORK/b104/_pkg_.a -trimpath $WORK/b104 -p net/http/httptrace
compile -o $WORK/b050/_pkg_.a -trimpath $WORK/b050 -p net/http
compile -o $WORK/b108/_pkg_.a -trimpath $WORK/b108 -p test_inline/handle -complete
compile -o $WORK/b107/_pkg_.a -trimpath $WORK/b107 -p net/http/httptest
compile -o $WORK/b048/_pkg_.a -trimpath $WORK/b048 -l -p github.com/houxl123/test_inline/test
compile -o ./_pkg_.a -trimpath $WORK/b001 -l -p main

There are quite a few compilations where -l is missing.

@cherrymui cherrymui changed the title cmd/compile: '-l' flag does not disable inlining for function with constant result cmd/go: '-gcflags=all=-l' flag does not disable inlining for all packages Sep 17, 2018

@bcmills

This comment has been minimized.

Copy link
Member

commented Sep 17, 2018

Reading the build log, -l is not applied to some of the compilation, including compiling handle.go.

Possibly the same underlying cause as #27681?

@cherrymui

This comment has been minimized.

Copy link
Contributor

commented Sep 17, 2018

With or without -c (for go test), I see the same set of packages compiled without -l.

@bcmills

This comment has been minimized.

Copy link
Member

commented Oct 24, 2018

Closing as a duplicate of #27681. Very likely the same cause, and we can always reopen if it turns out to be independent.

@bcmills bcmills closed this Oct 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.