alexbrainman / printer

Windows printing
BSD 3-Clause "New" or "Revised" License
227 stars 80 forks source link

panic when built with -gcflags=all=-d=checkptr #16

Closed jazzy-crane closed 4 years ago

jazzy-crane commented 4 years ago

Building cmd/print with the -gcflags=all=-d=checkptr option causes a panic when running print.exe -l

print.exe -l
fatal error: checkptr: unsafe pointer conversion

goroutine 1 [running]:
runtime.throw(0x50d846, 0x23)
        c:/go/src/runtime/panic.go:1116 +0x79 fp=0xc000107d20 sp=0xc000107cf0 pc=0x4332f9
runtime.checkptrAlignment(0xc000108000, 0x4dc5a0, 0x1)
        c:/go/src/runtime/checkptr.go:20 +0xd0 fp=0xc000107d50 sp=0xc000107d20 pc=0x406820
github.com/alexbrainman/printer.ReadNames(0xc000010109, 0xc000010120, 0xc00001c100, 0xc000107ee8, 0x45dd05)
        C:/MyGo/src/github.com/alexbrainman/printer/printer.go:145 +0xda fp=0xc000107e48 sp=0xc000107d50 pc=0x4c83ba
main.listPrinters(0xc00004a060, 0xc0000044b0)
        C:/MyGo/src/github.com/alexbrainman/printer/cmd/print/print.go:36 +0x3b fp=0xc000107f28 sp=0xc000107e48 pc=0x4ca07b
main.main()
        C:/MyGo/src/github.com/alexbrainman/printer/cmd/print/print.go:145 +0x197 fp=0xc000107f88 sp=0xc000107f28 pc=0x4caf67
runtime.main()
        c:/go/src/runtime/proc.go:203 +0x212 fp=0xc000107fe0 sp=0xc000107f88 pc=0x4359c2
runtime.goexit()
        c:/go/src/runtime/asm_amd64.s:1373 +0x1 fp=0xc000107fe8 sp=0xc000107fe0 pc=0x45dc41

I'm not 100% clear why though

jazzy-crane commented 4 years ago

I'm reaching a bit, but is this API fundamentally incompatible with cgo in this way as it returns pointers to within the memory allocated for buf?

jazzy-crane commented 4 years ago
diff --git a/printer.go b/printer.go
index ba07d0b..6cf27f4 100644
--- a/printer.go
+++ b/printer.go
@@ -126,6 +126,27 @@ func Default() (string, error) {
        return syscall.UTF16ToString(b), nil
 }

+const maxStringSize = 1 << 20
+
+func utf16PtrToString(p *uint16) string {
+       if p == nil {
+               return ""
+       }
+       // Find NUL terminator.
+       end := unsafe.Pointer(p)
+       n := 0
+       for *(*uint16)(end) != 0 {
+               end = unsafe.Pointer(uintptr(end) + unsafe.Sizeof(*p))
+               n++
+       }
+       if n >= maxStringSize {
+               // Error?
+               return ""
+       }
+       v := (*[maxStringSize]uint16)(unsafe.Pointer(p))[:n:n]
+       return syscall.UTF16ToString(v)
+}
+
 // ReadNames return printer names on the system
 func ReadNames() ([]string, error) {
        const flags = PRINTER_ENUM_LOCAL | PRINTER_ENUM_CONNECTIONS
@@ -142,11 +163,10 @@ func ReadNames() ([]string, error) {
                        return nil, err
                }
        }
-       ps := (*[1024]PRINTER_INFO_5)(unsafe.Pointer(&buf[0]))[:returned]
+       ps := (*[1024]PRINTER_INFO_5)(unsafe.Pointer(&buf[0]))[:returned:returned]
        names := make([]string, 0, returned)
        for _, p := range ps {
-               v := (*[1024]uint16)(unsafe.Pointer(p.PrinterName))[:]
-               names = append(names, syscall.UTF16ToString(v))
+               names = append(names, utf16PtrToString(p.PrinterName))
        }
        return names, nil
 }

Does seem to allow it to execute without panicking though. Still think this might be breaking CGO rules (can buf move in the memory space leaving all the pointers like p.PrinterName pointing to the wrong place?)

alexbrainman commented 4 years ago

I'm not 100% clear why though

I assume you use Go 1.15, and Go 1.15 changed unsafe.Pointer rules. See

The -race and -msan flags now always enable -d=checkptr, which checks uses of unsafe.Pointer. This was previously the case > on all OSes except Windows.

in https://golang.org/doc/go1.15#windows

So we need to adjust github.com/alexbrainman/printer code accordingly.

Do you think you can do that? I am happy to review your code. If not I can do it myself. I am not sure when I have time.

Thank you.

Alex

jazzy-crane commented 4 years ago

I actually built it with the flag manually as I'm trying to track down some possible runtime corruption in my project, though I now see it's a stroke of luck I did before upgrading to 1.15 !

alexbrainman commented 4 years ago

Here is the fix for this issue

https://github.com/alexbrainman/printer/tree/fix16

Please, verify.

Alex