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.

Sign up for GitHub

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

Jump to bottom

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

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

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

houxul opened this issue Sep 17, 2018 · 10 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.12

Comments

@houxul
Copy link

houxul commented Sep 17, 2018 •
edited

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
@houxul houxul mentioned this issue Sep 17, 2018
Closed
@bcmills
Copy link
Contributor

bcmills 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 added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 17, 2018
@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 has been minimized.

Sign in to view
@bcmills bcmills closed this as completed 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 has been minimized.

Sign in to view
@bcmills
Copy link
Contributor

bcmills commented Sep 17, 2018

(CC @bouk)

@bcmills
Copy link
Contributor

bcmills 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
Copy link
Contributor

bcmills 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
Copy link
Member

cherrymui commented Sep 17, 2018 •
edited

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
Copy link
Contributor

bcmills 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
Copy link
Member

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

@bcmills
Copy link
Contributor

bcmills commented Oct 24, 2018 •
edited

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 as completed Oct 24, 2018
@golang golang locked and limited conversation to collaborators Oct 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.12
Development

No branches or pull requests
4 participants
@bcmills @gopherbot @cherrymui @houxul