Skip to content

mime/multipart: With go 1.23.3, mime/multipart does not link #70679

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

Closed
testcheyi opened this issue Dec 4, 2024 · 7 comments
Closed

mime/multipart: With go 1.23.3, mime/multipart does not link #70679

testcheyi opened this issue Dec 4, 2024 · 7 comments

Comments

@testcheyi
Copy link

Go version

go version go1.23.3 windows/amd64

Output of go env in your module/workspace:

GO111MODULE=off
GOPATH=c:\go;c:\git\sasource\go\win\go;C:\git\sasource\jupiter\src\golib;C:\git\sasource\jupiter\src\golib\src\dg;C:\git\sasource\jupiter\src\golib\src;C:\git\sasource\jupiter\src\sputnik\;C:\git\sasource\dgagent\agent\common\cassini;c:\git\sasource\jupiter\src\golib\src\github.com;c:\git\sasource\jupiter\src\golib\src\gopkg.in;

What did you do?

I have a basic http client code that imports net/http package and an generate a GET request to a specific URL. It used to compile until i upgraded go to 1.23.3
I see that go made the linker change and documented as follows. I had to add -checklinkname=0 to all my project/cmake files to make our code compliant with 1.23.3

Linker
The linker now disallows using a //go:linkname directive to refer to internal symbols in the standard library (including the runtime) that are not marked with //go:linkname on their definitions. Similarly, the linker disallows references to such symbols from assembly code. For backward compatibility, existing usages of //go:linkname found in a large open-source code corpus remain supported. Any new references to standard library internal symbols will be disallowed.

A linker command line flag -checklinkname=0 can be used to disable this check, for debugging and experimenting purposes.

When building a dynamically linked ELF binary (including PIE binary), the new -bindnow flag enables immediate function binding.

What did you see happen?

$ go build httpclient.go

command-line-arguments

link: mime/multipart: invalid reference to net/textproto.readMIMEHeader

I had to add the following linker flags in order for building successfully.
$ go build -ldflags=-checklinkname=0 httpclient.go

What did you expect to see?

Linker
The linker now disallows using a //go:linkname directive to refer to internal symbols in the standard library (including the runtime) that are not marked with //go:linkname on their definitions. Similarly, the linker disallows references to such symbols from assembly code. For backward compatibility, existing usages of //go:linkname found in a large open-source code corpus remain supported. Any new references to standard library internal symbols will be disallowed.

A linker command line flag -checklinkname=0 can be used to disable this check, for debugging and experimenting purposes.

When building a dynamically linked ELF binary (including PIE binary), the new -bindnow flag enables immediate function binding.

@gabyhelp
Copy link

gabyhelp commented Dec 4, 2024

Related Issues

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@seankhliao
Copy link
Member

this is working as intended.

@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Dec 4, 2024
@ianlancetaylor
Copy link
Member

The mime/multipart package does have a linkname. But it should be permitted. Please tell us exactly what your input program looks like and exactly how you built it.

@testcheyi
Copy link
Author

testcheyi commented Dec 6, 2024

Following is a sample/simplified code to demonstrate the problem.

$ cat h.go
package main

import (
        "crypto/tls"
        "fmt"
        "io/ioutil"
        "net/http"
        "os"

        "golang.org/x/net/http2"
)

func main() {

        c := &tls.Config{}
        tr := &http.Transport{
                TLSClientConfig: c,
        }
        if err := http2.ConfigureTransport(tr); err != nil {
                panic("error configuring transport for h2")
        }

        req, err := http.NewRequest("GET", os.Args[1], nil)
        if err != nil {
                fmt.Printf("Failed generating request %v\n", os.Args[1])
                return
        }

        resp, err := tr.RoundTrip(req)
        if err != nil {
                fmt.Printf("Failed sending request %v with err %v \n", os.Args[1], err)
                return
        }
        if resp != nil {
                _, _ = ioutil.ReadAll(resp.Body)
        }
        if resp.StatusCode == 502 {
                // failed to proxy request upstream
                // "http2: client conn not usable"
                fmt.Printf("Received error response %v to %v\n", resp.StatusCode, os.Args[1])
                return
        }
}

$ go build h.go **<<<<<FAILED**
# command-line-arguments
link: mime/multipart: invalid reference to net/textproto.readMIMEHeader

garaveti@garaveti-p5560 MINGW64 /c/dg/httpclient
$ go build  -ldflags=-checklinkname=0 h.go **<<<SUCCESSFUL**

Above code (simple http client code) builds with go1.22.2. Now go.1.23.3 requires additional ldflags. This forces every one who is using net/http to add this ldflag to their make files. It is not what is stated in the documentation which is as follows. Am I misunderstanding.

For backward compatibility, existing usages of //go:linkname found in a large open-source code corpus remain supported. Any new references to standard library internal symbols will be disallowed.

@ianlancetaylor
Copy link
Member

Thanks for the test case. Unfortunately, I can't recreate the problem. Your test case builds for me with Go 1.23.3 with no error on both Linux and Windows.

In general a reference using go:linkname is permitted if there is an explicit go:linkname in the target library. And in net/textproto/reader.go I see

// readMIMEHeader is accessed from mime/multipart.
//go:linkname readMIMEHeader

Do you have that in your Go 1.23.3 copy of net/textproto/reader.go? If you do I don't understand why you would be getting this error.

@testcheyi
Copy link
Author

Thanks @ianlancetaylor. We have some custom changes in reader.go and when i merged go.1.23.3, the comment got lost. Hence the linking issue. After inserting above comments back, the code compiles. Sorry for the trouble.

@ianlancetaylor
Copy link
Member

Thanks for following up.

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

No branches or pull requests

4 participants