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

Builds fails on 32 bit systems #707

Closed
misthios opened this issue May 22, 2024 · 9 comments
Closed

Builds fails on 32 bit systems #707

misthios opened this issue May 22, 2024 · 9 comments

Comments

@misthios
Copy link

Hello,

I was updating the minify package in the Alpine Linux repos from 1.20.24 to 1.20.28 when I noticed that most if not all 32bit systems failed (log: https://gitlab.alpinelinux.org/Misthios/aports/-/jobs/1394860 ) with the same error:

# github.com/tdewolff/minify/v2/bindings/js
bindings/js/minify.go:28:12: invalid array length 1 << 32 (untyped int constant 4294967296)
bindings/js/minify.go:37:12: invalid array length 1 << 32 (untyped int constant 4294967296)
# github.com/tdewolff/minify/v2/bindings/py
bindings/py/minify.go:28:12: invalid array length 1 << 32 (untyped int constant 4294967296)
bindings/py/minify.go:37:12: invalid array length 1 << 32 (untyped int constant 4294967296)

I have made some changes to those file to get the build passing (log: https://gitlab.alpinelinux.org/Misthios/aports/-/jobs/1395003 ).
But since those changed are untested since I do not use those bindings.

Here are the changes:

--- /bindings/py/minify.go
+++ /bindings/py/minify.go
@@ -25,7 +25,10 @@
 }
 
 func goBytes(str *C.char, length C.longlong) []byte {
-	return (*[1 << 32]byte)(unsafe.Pointer(str))[:length:length]
+	unsafePtr := unsafe.Pointer(str)
+	byteSlice := unsafe.Slice((*byte)(unsafePtr), length)
+
+	return byteSlice
 }
 
 func goStringArray(carr **C.char, length C.longlong) []string {
@@ -34,11 +37,12 @@
 	}
 
 	strs := make([]string, length)
-	arr := (*[1 << 32]*C.char)(unsafe.Pointer(carr))[:length:length]
 	for i := 0; i < int(length); i++ {
-		strs[i] = C.GoString(arr[i])
+		cstr := *(**C.char)(unsafe.Pointer(uintptr(unsafe.Pointer(carr)) + uintptr(i)*unsafe.Sizeof(*carr)))
+		strs[i] = C.GoString(cstr)
 	}
 	return strs
+
 }
 
 //export minifyConfig
--- /bindings/js/minify.go
+++ /bindings/js/minify.go
@@ -25,7 +25,11 @@
 }
 
 func goBytes(str *C.char, length C.longlong) []byte {
-	return (*[1 << 32]byte)(unsafe.Pointer(str))[:length:length]
+	unsafePtr := unsafe.Pointer(str)
+	byteSlice := unsafe.Slice((*byte)(unsafePtr), length)
+	
+	return byteSlice
+
 }
 
 func goStringArray(carr **C.char, length C.longlong) []string {
@@ -34,11 +38,13 @@
 	}
 
 	strs := make([]string, length)
-	arr := (*[1 << 32]*C.char)(unsafe.Pointer(carr))[:length:length]
 	for i := 0; i < int(length); i++ {
-		strs[i] = C.GoString(arr[i])
+		cstr := *(**C.char)(unsafe.Pointer(uintptr(unsafe.Pointer(carr)) + uintptr(i)*unsafe.Sizeof(*carr)))
+		strs[i] = C.GoString(cstr)
 	}
+
 	return strs
+
 }
 
 //export minifyConfig
@tdewolff
Copy link
Owner

Thanks for the heads up! I believe we can just convert the 32 to 31, it's an upper limit on how long a slice can be (which is actually 1<<31 for 32 bit systems since Len() returns an int).

@tdewolff
Copy link
Owner

Can you try v2.20.29 and tell me if it works?

@misthios
Copy link
Author

Hello,

the new versions gives the same error type but different value

# github.com/tdewolff/minify/v2/bindings/py
bindings/py/minify.go:28:12: invalid array length 1 << 31 (untyped int constant 2147483648)
bindings/py/minify.go:37:12: invalid array length 1 << 31 (untyped int constant 2147483648)
# github.com/tdewolff/minify/v2/bindings/js
bindings/js/minify.go:28:12: invalid array length 1 << 31 (untyped int constant 2147483648)
bindings/js/minify.go:37:12: invalid array length 1 << 31 (untyped int constant 2147483648)

@tdewolff
Copy link
Owner

Off-by-one error, please check v2.20.30!

@misthios
Copy link
Author

Hello,

the 32bit build is unfortunately still broken.

  1. the 2.20.30 commit only fixes one instance of the previous problem:
the goStringArray has the new -1, the goBytes still has 31 
  1. after patching 1 we get a new problem which can be seen in the screenshot below (i only patched the python bindings for problem 1).

image

@tdewolff
Copy link
Owner

Must have been late, how does v2.20.31 do?

@misthios
Copy link
Author

Hello,

I still get the same result unfortunately:
image

@tdewolff
Copy link
Owner

I really hope this is now fixed in v2.20.32, I have no idea why the address space for 32-bit systems is not math.MaxInt32-1 == 1<<31 - 1...

@misthios
Copy link
Author

Hello,

2.20.32 seems to be working

Thank you :)

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

2 participants