Repository navigation
fix: restore dead-method elimination with obfuscated reflect - #1157
Merged
Merged
Conversation
A constant MethodByName lookup should retain only the named method, but obfuscating reflect currently makes the linker retain unrelated exported methods as well. Record the current behavior in the existing reflection fixture with a separate minimal program, so its other dynamic reflection calls cannot mask the regression. The correct assertion fails on master with: unexpected match for ["unused-method-marker"] in methods-bin Refs burrowers#1156. Co-authored-by: Andrey Pshenkin <andrey.pshenkin@gmail.com>
The compiler exempts reflect own Method implementations by exact receiver-qualified symbol names. Obfuscating rtype and interfaceType breaks that exemption, so the linker sees reflection and retains every exported method of reachable types, even for constant MethodByName calls. Preserve both receiver names and assert that an unrelated method body is absent while the selected method still executes. Exercise interface lookups too; removing interfaceType preservation fails the same check. The original code fails with: unexpected match for ["unused-method-marker"] in methods-bin Also preserve Value, as proposed in burrowers#1150 for burrowers#1149. Restoring method elimination without recognizing genuine Value.Method callers can expose unreachable-method crashes previously masked by excessive retention. Fixes burrowers#1156. Co-authored-by: Andrey Pshenkin <andrey.pshenkin@gmail.com>
Preserving reflect receiver names exposed unconditional receiver hashing in linkname rewriting. The existing linkname fixture then failed with: relocation target reflect.(*rZMjqBiHF6Ld).NumMethod not defined Apply the toolchain-name exemption to both pointer and value receiver spellings while continuing to obfuscate ordinary receivers and method names. The existing rtype links cover the pointer case; add a linknamestd call to Value.IsValid to exercise the value case in the same build. Refs burrowers#1156. Co-authored-by: Andrey Pshenkin <andrey.pshenkin@gmail.com>
luames
marked this pull request as ready for review
October 6, 2026 21:59
luantak
approved these changes
Oct 6, 2026
This was referenced Oct 6, 2026
Closed
luames
pushed a commit
to luames/garble
that referenced
this pull request
Oct 8, 2026
The reflect.Value name preservation for burrowers#1149 landed in burrowers#1157, but Daniel Martí's direct Value.Method(...).Call regression from burrowers#1150 was not included. Exercise the call in the existing reflection program and add its output to the shared golden file. This reuses both the garbled build and the plain Go comparison without adding a separate build. Retain the upstream reflected-recovery and method-elimination assertions. Fixes burrowers#1149. Co-authored-by: Daniel Martí <mvdan@mvdan.cc>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1156. The compiler's
usemethodexemption matches reflect's own method implementations by receiver-qualified symbol name. RenamingrtypeandinterfaceTypebreaks that exemption, marks those implementations withAttrReflectMethod, and makes the linker retain every exported method of reachable types.Preserve both receiver names. This also includes the
Valuepreservation from #1150 for #1149, since restoring dead-method elimination without it can expose unreachable-method crashes in genuine reflection callers. The PR targets master and does not include the other fixes from #1150.Extend the existing reflection fixture with a separate minimal program that looks up a constant method name on concrete and interface types, executes the concrete method, and checks that an unrelated method's marker is absent. Keeping this separate from the fixture's main program prevents its dynamic reflection calls from legitimately retaining every method. The test commit records the old behavior; the fix commit flips the assertion.
Linkname rewriting also respects preserved receiver names for both pointer and value methods. The existing
rtypelinknames and an addedValue.IsValidcall cover those cases.@APshenkin is credited as a co-author on all commits for the diagnosis, reproducer, and proposed fix.
Tests
go test -run '^TestScript$/^(reflect|linkname)$' -count=1passes without short mode, including the plain-build comparisons.interfaceTypepreservation also makes it fail.go vet ./...andgit diff --checkpass. Changed Go source and archived Go entries are gofmt-clean.Implemented by Hermes Agent using gpt-6.1-sol, acting on behalf of @luantak.