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

runtime: redirect println(runtime.writeErr) to NSLog(asl_log) on iOS #19744

Open
bronze1man opened this issue Mar 28, 2017 · 14 comments
Open

runtime: redirect println(runtime.writeErr) to NSLog(asl_log) on iOS #19744

bronze1man opened this issue Mar 28, 2017 · 14 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Proposal Proposal-Accepted
Milestone

Comments

@bronze1man
Copy link
Contributor

bronze1man commented Mar 28, 2017

Please answer these questions before submitting your issue. Thanks!

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

go version go1.7.5 darwin/amd64

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH=""
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/m9/qtbxkp6s3p96fk54rln7qhj80000gp/T/go-build412033723=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"

What did you do?

Get a uncatch panic on ios in network extension process.As I can not attach xcode to that process.I can only use devices window of xcode to view the NSLog output from that process.

What did you expect to see?

See panic message in NSLog output of that network extension process.

What did you see instead?

Nothing.

@bronze1man
Copy link
Contributor Author

bronze1man commented Mar 28, 2017

I managed to implement this feature with go1.7.5 .
I want someone who is familiar with runtime and cgo package to review my code.
So that I can sure they are correct in all situation.

add /usr/local/go/src/runtime/cgo/write_err_ios.go

// +build ios
// +build arm arm64

package cgo

/*
#include <asl.h>

void asl_log_wrap(const char *str) {
	asl_log(NULL, NULL, ASL_LEVEL_NOTICE, "%s", str);
}
*/
import "C"

add /usr/local/go/src/runtime/write_err_ios.go

// +build ios
// +build arm arm64

package runtime

import "unsafe"

var (
	gWriteBuf [1024]byte
	gWritePos int
)

//go:nosplit
func asmAslLogWrap(b unsafe.Pointer)

//go:linkname writeErr writeErr
func writeErr(b []byte) {
	// Write to stderr for command-line programs.
	write(2, unsafe.Pointer(&b[0]), int32(len(b)))

	for _, v := range b {
		if v == 0 {
			v = '0'
		}
		gWriteBuf[gWritePos] = v
		gWritePos++
		if v == '\n' || gWritePos == len(gWriteBuf)-1 {
			gWriteBuf[gWritePos] = 0
			asmAslLogWrap(unsafe.Pointer( &gWriteBuf[0] ))
			gWritePos = 0
		}
	}
}

//go:cgo_import_static asl_log_wrap

add /usr/local/go/src/runtime/write_err_ios_arm.s

// +build ios,arm

#include "go_asm.h"
#include "go_tls.h"
#include "funcdata.h"
#include "textflag.h"

TEXT runtime·asmAslLogWrap(SB),NOSPLIT,$-4
    MOVW 4(R13),R0 
    SUB $32,R13
    MOVW g,16(R13)
    MOVW R14,24(R13)

    BL asl_log_wrap(SB)

    MOVW 24(R13),R14
    MOVW 16(R13),g
    ADD $32,R13
	RET

add /usr/local/go/src/runtime/write_err_ios_arm64.s

// +build ios,arm64

#include "go_asm.h"
#include "go_tls.h"
#include "funcdata.h"
#include "textflag.h"

TEXT runtime·asmAslLogWrap(SB),NOSPLIT,$-8
    MOVD 8(RSP),R0 
    SUB $32,RSP
    MOVD g,16(RSP)
    MOVD R30,24(RSP)

    BL asl_log_wrap(SB)

    MOVD 24(RSP),R30
    MOVD 16(RSP),g
    ADD $32,RSP
	RET

change /usr/local/go/src/runtime/write_err.go

// Copyright 2009 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

// +build !android
// +build !ios !arm,!arm64

package runtime

import "unsafe"

func writeErr(b []byte) {
	write(2, unsafe.Pointer(&b[0]), int32(len(b)))
}

I do not use NSLog, because NSLog will output its output to stderr. You will see two same message in xcode output window.
i do not add this feature to ios x86, because network extension process can not run on a ios simulator.
I do not send pr, because you guys do not allow it, and your guys may not accept this one. I just need it works right now.
I do not try apply this into go1.8, because I also have other modifies to my own version of go1.7.5.I will send more Proposals with change set to discuss those modifies with your guys later.

@gopherbot gopherbot added this to the Proposal milestone Mar 28, 2017
@bradfitz
Copy link
Contributor

bradfitz commented Mar 28, 2017

/cc @eliasnaur @crawshaw

@eliasnaur
Copy link
Contributor

eliasnaur commented Mar 28, 2017

Seems reasonable. Android already redirects its output to android_log_print. @bronze1man would you like to propose your changes as a CL?

@bronze1man
Copy link
Contributor Author

bronze1man commented Mar 28, 2017

@eliasnaur
Sorry,I also have other patches to my own version of go1.7.5.
If I do not have them, my toolbox do not work.I do not know how to port one patch to go1.8. So I can not port this one to go1.8.I can not test them with go1.8 and ios right now.I do not know how to write/run golang code in ios without my toolbox right now.There is a lot of detail about cgo/compiler stuff that I have forgot.

Is it ok to send a CL to go1.7.5?

@bradfitz
Copy link
Contributor

bradfitz commented Mar 28, 2017

Is it ok to send a CL to go1.7.5?

No. It's not even okay to send a CL to Go 1.8. You need to send a change to Go tip.

See https://golang.org/doc/contribute.html

@bronze1man
Copy link
Contributor Author

bronze1man commented Mar 28, 2017

@bradfitz
Looks like I need send another two propose/patches first, then this one.
Or other guys can help me test that code in Go tip and send a CL.

I managed to port #19745 to go1.8, but do not found a way to port #19746 to go1.8.

@rsc
Copy link
Contributor

rsc commented Apr 3, 2017

Accepting based on discussion (Elias is in favor, Android already does this).

@rsc rsc modified the milestones: Unreleased, Proposal Apr 3, 2017
@rsc rsc changed the title proposal: ios: redirect println(runtime.writeErr) to NSLog(asl_log) runtime: redirect println(runtime.writeErr) to NSLog(asl_log) on iOS Apr 3, 2017
@bronze1man
Copy link
Contributor Author

bronze1man commented May 5, 2017

change set that works on go1.8(it is not the same as go1.7)

diff --git a/src/runtime/cgo/write_err_ios.c b/src/runtime/cgo/write_err_ios.c
new file mode 100644
index 0000000..452f525
--- /dev/null
+++ b/src/runtime/cgo/write_err_ios.c
@@ -0,0 +1,8 @@
+// +build ios
+// +build arm arm64
+
+#include <asl.h>
+
+void x_cgo_asl_log_wrap(const char *str) {
+	asl_log(NULL, NULL, ASL_LEVEL_NOTICE, "%s", str);
+}
\ No newline at end of file
diff --git a/src/runtime/cgo/write_err_ios.go b/src/runtime/cgo/write_err_ios.go
new file mode 100644
index 0000000..b9c4e67
--- /dev/null
+++ b/src/runtime/cgo/write_err_ios.go
@@ -0,0 +1,10 @@
+// +build ios
+// +build arm arm64
+
+package cgo
+
+import _ "unsafe"
+
+//go:cgo_import_static x_cgo_asl_log_wrap
+//go:linkname x_cgo_asl_log_wrap runtime.x_cgo_asl_log_wrap
+var x_cgo_asl_log_wrap byte
\ No newline at end of file
diff --git a/src/runtime/write_err.go b/src/runtime/write_err.go
index 6b1467b..73ed4ba 100644
--- a/src/runtime/write_err.go
+++ b/src/runtime/write_err.go
@@ -3,6 +3,7 @@
 // license that can be found in the LICENSE file.
 
 // +build !android
+// +build !ios !arm,!arm64
 
 package runtime
 
diff --git a/src/runtime/write_err_ios.go b/src/runtime/write_err_ios.go
new file mode 100644
index 0000000..059e6fd
--- /dev/null
+++ b/src/runtime/write_err_ios.go
@@ -0,0 +1,37 @@
+// +build ios
+// +build arm arm64
+
+package runtime
+
+import "unsafe"
+
+var (
+	gWriteBuf [1024]byte
+	gWritePos int
+)
+
+//go:nosplit
+func asmAslLogWrap(b unsafe.Pointer)
+
+//go:linkname writeErr writeErr
+func writeErr(b []byte) {
+	// Write to stderr for command-line programs.
+	write(2, unsafe.Pointer(&b[0]), int32(len(b)))
+
+	for _, v := range b {
+		if v == 0 {
+			v = '0'
+		}
+		gWriteBuf[gWritePos] = v
+		gWritePos++
+		if v == '\n' || gWritePos == len(gWriteBuf)-1 {
+			gWriteBuf[gWritePos] = 0
+			asmAslLogWrap(unsafe.Pointer( &gWriteBuf[0] ))
+			gWritePos = 0
+		}
+	}
+}
+
+// define in src/runtime/cgo/write_err_ios.go:10 and /usr/local/go/src/runtime/cgo/write_err_ios.c
+//go:linkname x_cgo_asl_log_wrap x_cgo_asl_log_wrap
+var x_cgo_asl_log_wrap unsafe.Pointer
\ No newline at end of file
diff --git a/src/runtime/write_err_ios_arm.s b/src/runtime/write_err_ios_arm.s
new file mode 100644
index 0000000..57be68e
--- /dev/null
+++ b/src/runtime/write_err_ios_arm.s
@@ -0,0 +1,21 @@
+// +build ios,arm
+
+#include "go_asm.h"
+#include "go_tls.h"
+#include "funcdata.h"
+#include "textflag.h"
+
+TEXT runtime·asmAslLogWrap(SB),NOSPLIT,$-4
+    MOVW 4(R13),R0 // 第一个参数
+    SUB $32,R13
+    MOVW g,16(R13)
+    MOVW R14,24(R13)
+
+    BL x_cgo_asl_log_wrap(SB)
+
+    MOVW 24(R13),R14
+    MOVW 16(R13),g
+    ADD $32,R13
+	RET
+
+
diff --git a/src/runtime/write_err_ios_arm64.s b/src/runtime/write_err_ios_arm64.s
new file mode 100644
index 0000000..adcd844
--- /dev/null
+++ b/src/runtime/write_err_ios_arm64.s
@@ -0,0 +1,21 @@
+// +build ios,arm64
+
+#include "go_asm.h"
+#include "go_tls.h"
+#include "funcdata.h"
+#include "textflag.h"
+
+TEXT runtime·asmAslLogWrap(SB),NOSPLIT,$-8
+    MOVD 8(RSP),R0 // 第一个参数
+    SUB $32,RSP
+    MOVD g,16(RSP)
+    MOVD R30,24(RSP)
+
+    BL x_cgo_asl_log_wrap(SB)
+
+    MOVD 24(RSP),R30
+    MOVD 16(RSP),g
+    ADD $32,RSP
+	RET
+
+

@eliasnaur
Copy link
Contributor

eliasnaur commented May 5, 2017

Thank you very much. Changes to Go needs to go through a review process before they can be considered. If you submit a CL according to https://golang.org/doc/contribute.html I'll make sure it gets reviewed.

Also note that we've entered the freeze stage for Go 1.9. The earliest a change like this can go in is for Go 1.10.

@odeke-em
Copy link
Member

odeke-em commented Oct 2, 2019

Hello again @bronze1man, please send CLs to Gerrit as @eliasnaur noted, the Go1.13 tree has been open for a while. The sooner the CL is made the sooner review feedback will roll in but also the less likely it will be to stall :)

@tmm1
Copy link
Contributor

tmm1 commented Mar 30, 2022

Would really like to see this pushed forward.

Looks like os_log replaced asl_log, so it may make more sense to wrap and use that instead.

cc @nickoneill


Although sending panic output to NSLog is a definite improvement, it still doesn't help debug crashes in production. Some pluggable way to get the crash output to the app, so it can report it via Sentry or other crash reporter would be ideal.

(Perhaps the simplest interface is a global for a file descriptor to write into, which can default to stderr, but be overridden by the wrapping app to point to a file instead? Then the contents of the file could be attached to any crash reports.)

@tmm1
Copy link
Contributor

tmm1 commented Mar 30, 2022

Here's what I ended up doing:

Author: Aman Karmani <aman@tmm1.net>
Date:   Wed Mar 30 13:56:00 2022 -0700

    runtime: on iOS builds, allow redirecting panic output

diff --git a/src/runtime/write_err.go b/src/runtime/write_err.go
index 81ae872e9c..1cb5174abd 100644
--- a/src/runtime/write_err.go
+++ b/src/runtime/write_err.go
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.

-//go:build !android
+//go:build !android && !ios

 package runtime

diff --git a/src/runtime/write_err_ios.go b/src/runtime/write_err_ios.go
new file mode 100644
index 0000000000..3526d3d5a1
--- /dev/null
+++ b/src/runtime/write_err_ios.go
@@ -0,0 +1,15 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package runtime
+
+import "unsafe"
+
+var (
+       WriteErrFD uintptr = 2
+)
+
+func writeErr(b []byte) {
+       write(WriteErrFD, unsafe.Pointer(&b[0]), int32(len(b)))
+}

@ianlancetaylor
Copy link
Contributor

ianlancetaylor commented Mar 30, 2022

See #42888. Can the API described there be used to solve this issue?

@tmm1
Copy link
Contributor

tmm1 commented Mar 30, 2022

It seems like there are two different concerns:

  1. integration into system logging
  2. integration into crash reporting

For (2), I do think #42888 is what I'm looking for.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 7, 2022
@prattmic prattmic modified the milestones: Unreleased, Backlog Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Proposal Proposal-Accepted
Projects
Status: Todo
Development

No branches or pull requests

9 participants