Field names are obfuscated with the identity of their containing struct as
a salt, which must be stable across packages so that converting between
identical struct types keeps working. Hashing field types into that salt
breaks for an anonymous struct{F Q} returned by a generic function: the
declaring package hashes the type parameter Q while a consuming package only
sees an instantiation such as struct{F int}, with no origin to recover Q.
The previous commit fixed this for a named generic field type, but not for
a bare type parameter.
Drop field types and fold in the field position instead, which is stable
across instantiations and conversions yet still keeps reordered structs
distinct. This only makes more fields share a salt, which is harmless as
non-identical structs are never converted between one another.
Since field types are no longer hashed, the salt never recurses into the
rest of the type graph, so the type-argument NOTE(garble) tweak to the
*types.Named case added by the previous commit is now dead code; revert it
to keep the bundled hasher faithful to upstream.
Field names are salted with the identity of their containing struct, and
the salt must be stable across instantiations. For named generic types we
recover the generic origin, but an anonymous struct returned by a generic
function, e.g. struct{F G[T]}, carries no origin info: the declaring
package hashes it with the type parameter T, while another package only
sees the instantiated struct{F G[int]}. Hashing the type arguments then
gave the field two different names and a broken build.
Drop type arguments from the field-name salt to keep it stable. This only
makes more fields share a salt, which is harmless as non-identical structs
are never converted between one another.
Thanks to David Roizenman for providing the test case.
Fixes#1041.
The marshaler interface and some of the other types were unnecessary.
The remaining test still fails without the fix,
because the crux of the issue is the separate package
with the Result generic type having Data as a type parameter and field.
When compiling a package that instantiates a generic type from another
package, info.Types only contains the instantiated *types.Named. The
*types.Struct case in recordFieldToStruct returns early on instantiated
structs (field != field.Origin()), so the origin struct was never
recorded in fieldToStruct. At rewrite time, looking up the origin field
then failed with "could not find struct for field X".
Fix by also recursing into typ.Origin() for instantiated named types,
so the uninstantiated struct is visited and its fields are recorded.
Fixes#1027.
Use `types.Var.Origin()` when mapping and hashing struct fields so selector and
declaration sites share the same canonical field identity. This fixes build
failures for generic aliases and named types over those aliases.
Fixes#924
Avoid hashing free type parameters by object/name identity in type
hashing and use traversal-local canonical IDs instead.
This keeps obfuscated field names stable across alpha-renamed generic
signatures (e.g. T vs newT), fixing interface/method mismatches.
The added test case fails without the fix:
> exec garble build
[stderr]
# test/main
YiaBFlNH.go:48: cannot use bT6psGs5O (variable of type *QdwRyqV[aEqmF4M, zpotHfQdnNB]) as E_OKfdHoPH[zpotHfQdnNB] value in return statement: *QdwRyqV[aEqmF4M, zpotHfQdnNB] does not implement E_OKfdHoPH[zpotHfQdnNB] (wrong type for method Redirect)
have Redirect(struct{palTavb zpotHfQdnNB})
want Redirect(struct{g3N8A_R zpotHfQdnNB})
The fix is Paul's; this is rebased on an updated version of typeutil,
and the test is now part of typeparams.txtar and much smaller.
Fixes#991.
Co-authored-by: Paul Scheduikat <lu4p@pm.me>
We only did this for Container in the type switch, but not for Struct.
The added test case panics otherwise.
Just like in the previous case, we still don't need to recurse
into type parameters for fieldToStruct to be filled correctly.
Fixes#899
This lets us start taking advantage of featurs from Go 1.23,
particularly tracking aliases in go/types and iterators.
Note that we need to add code to properly handle or skip over the new
*types.Alias type which go/types produces for Go type aliases.
Also note that we actually turn this mode off entirely for now,
due to the bug reported at https://go.dev/issue/70394.
We don't yet remove our own alias tracking code yet due to the above.
We hope to be able to remove it very soon.
Go 1.21.0 was released in August 2023, so our upcoming release
will no longer support the Go 1.20 release series.
The first Go 1.22 release candidate is also due in December 2023,
less than a month from now, so dropping 1.20 will simplify 1.22 work.
The first makes our test scripts more consistent, as all external
program executions happen via "exec" and are not as easily confused
with custom builtin commands like our "generate-literals".
The second catches mistakes if any of our txtar files have duplicate
files, where all but one of the contents would be ignored before.
We can drop the code that kicked in when GOGARBLE was empty.
We can also add the value in addGarbleToHash unconditionally,
as we never allow it to be empty.
In the tests, remove all GOGARBLE lines where it just meant "obfuscate
everything" or "obfuscate the entire main module".
cgo.txtar had "obfuscate everything" as a separate step,
so remove it entirely.
linkname.txtar started failing because the imported package did not
import strings, so listPackage errored out. This wasn't a problem when
strings itself wasn't obfuscated, as transformLinkname silently left
strings.IndexByte untouched. It is a problem when IndexByte does get
obfuscated. Make that kind of listPackage error visible, and fix it.
reflect.txtar started failing with "unreachable method" runtime throws.
It's not clear to me why; it appears that GOGARBLE=* makes the linker
think that ExportedMethodName is suddenly unreachable.
Work around the problem by making the method explicitly reachable,
and leave a TODO as a reminder to investigate.
Finally, gogarble.txtar no longer needs to test for GOPRIVATE.
The rest of the test is left the same, as we still want the various
values for GOGARBLE to continue to work just like before.
Fixes#594.
Following the best practices from upstream.
In particular, the "txt" extension is somewhat ambiguous.
This may cause some conflicts due to the git diff noise,
but hopefully we won't ever do this again.
Daniel Martí