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

Weird error with latest garble #573

Closed
VeNoMouS opened this issue Aug 28, 2022 · 1 comment · Fixed by #576
Closed

Weird error with latest garble #573

VeNoMouS opened this issue Aug 28, 2022 · 1 comment · Fixed by #576
Assignees
@mvdan

Comments

@VeNoMouS
Copy link

This issue did not occur on earlier garble builds..

# github.com/andybalholm/brotli
:24: syntax error: unexpected ., expecting }
:10: syntax error: non-declaration statement outside function body
:2: syntax error: non-declaration statement outside function body
:10: syntax error: non-declaration statement outside function body
:12: syntax error: non-declaration statement outside function body
:2: syntax error: non-declaration statement outside function body
:25: syntax error: unexpected ., expecting }
:4: syntax error: non-declaration statement outside function body
:11: syntax error: non-declaration statement outside function body
:11: too many errors
exit status 2
exit status 2
@mvdan
Copy link
Member

mvdan commented Aug 31, 2022

Thanks for reporting, I can reproduce on master.

@mvdan mvdan self-assigned this Aug 31, 2022
mvdan added a commit to mvdan/garble-fork that referenced this issue Aug 31, 2022
@mvdan
work around another go/printer bug to fix andybalholm/brotli
6d43bda 
When obfuscating the following piece of code:

	func issue_573(s struct{ f int }) {
		var _ *int = &s.f
		/*x*/
	}

the function body would roughly end up printed as:
we would roughly end up with:

	var _ *int = &dZ4xYx3N
	/*x*/.rbg1IM3V

Note that the /*x*/ comment got moved earlier in the source code.
This happens because the new identifiers are longer, so the printer
thinks that the selector now ends past the comment.

That would be fine - we don't really mind where comments end up,
because these non-directive comments end up being removed anyway.

However, the resulting syntax is wrong, as the period for the selector
must be on the first line rather than the second.
This is a go/printer bug that we should fix upstream,
but until then, we must work around it in Go 1.18.x and 1.19.x.

The fix is somewhat obvious in hindsight. To reduce the chances that
go/printer will trip over comments and produce invalid syntax,
get rid of most comments before we use the printer.
We still keep the removal of comments after printing,
since go/printer consumes some comments in ast.Node Doc fields.

Add the minimized unit test case above, and add the upstream project
that found this bug to check-third-party.
andybalholm/brotli helps cover a compression algorithm and ccgo code
generation from C to Go, and it's also a fairly popular module,
particular with HTTP implementations which want pure-Go brotli.

While here, fix the check-third-party script: it was setting GOFLAGS
a bit too late, so it may run `go get` on the wrong mod file.

Fixes burrowers#573.
@mvdan mvdan mentioned this issue Aug 31, 2022
Merged
@mvdan mvdan closed this as completed in #576 Sep 2, 2022
mvdan added a commit that referenced this issue Sep 2, 2022
@mvdan
work around another go/printer bug to fix andybalholm/brotli
60dbece 
When obfuscating the following piece of code:

	func issue_573(s struct{ f int }) {
		var _ *int = &s.f
		/*x*/
	}

the function body would roughly end up printed as:
we would roughly end up with:

	var _ *int = &dZ4xYx3N
	/*x*/.rbg1IM3V

Note that the /*x*/ comment got moved earlier in the source code.
This happens because the new identifiers are longer, so the printer
thinks that the selector now ends past the comment.

That would be fine - we don't really mind where comments end up,
because these non-directive comments end up being removed anyway.

However, the resulting syntax is wrong, as the period for the selector
must be on the first line rather than the second.
This is a go/printer bug that we should fix upstream,
but until then, we must work around it in Go 1.18.x and 1.19.x.

The fix is somewhat obvious in hindsight. To reduce the chances that
go/printer will trip over comments and produce invalid syntax,
get rid of most comments before we use the printer.
We still keep the removal of comments after printing,
since go/printer consumes some comments in ast.Node Doc fields.

Add the minimized unit test case above, and add the upstream project
that found this bug to check-third-party.
andybalholm/brotli helps cover a compression algorithm and ccgo code
generation from C to Go, and it's also a fairly popular module,
particular with HTTP implementations which want pure-Go brotli.

While here, fix the check-third-party script: it was setting GOFLAGS
a bit too late, so it may run `go get` on the wrong mod file.

Fixes #573.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mvdan mvdan

Labels
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

work around another go/printer bug to fix andybalholm/brotli mvdan/garble-fork
2 participants
@VeNoMouS @mvdan