debugdir.txtar also needed tweaking as runtime/map.go is gone
starting in Go 1.24.
Finally, modinfo.txtar needed tweaking since Go 1.24 started stamping
Go binaries with VCS-derived module versions, so we no longer end up
with empty "(devel)" versions.
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
Allows us to remove EmbeddedAliasFields, recordedObjectString,
and all the logic around using them.
Resolves an issue where a user was running into a panic in
our logic to record embedded aliases.
Note that this means we require Go 1.23.5 or later now,
which also meant some changes to goversion.txtar to keep it green.
Fixes#827.
obfuscatedImportPath already handled ToObfuscate, so the callers
don't have to do that as well. Handle main packages too, whose logic
was sprinkled and repeated throughout the project.
Reflection can show package names alone via reflect.Type.String,
and I'm sure they are available in other ways.
We were obfuscating "package p" exactly like the import path "foo.com/p"
which worked OK for the most part, but not for reflection.
This is also a slight improvement to the quality of the obfuscation,
given that package names and import paths will no longer be aligned
when obfuscated.
There's no need to reach for sharedCache.ListedPackages
when the caller is computePkgCache, which already has the lpkg value.
While here, compare *go/types.Package pointers directly
rather than via their import path strings.
Go code can retrieve and use field and method names via the `reflect` package.
For that reason, historically we did not obfuscate names of fields and methods
underneath types that we detected as used for reflection, via e.g. `reflect.TypeOf`.
However, that caused a number of issues. Since we obfuscate and build one package
at a time, we could only detect when types were used for reflection in their own package
or in upstream packages. Use of reflection in downstream packages would be detected
too late, causing one package to obfuscate the names and the other not to, leading to a build failure.
A different approach is implemented here. All names are obfuscated now, but we collect
those types used for reflection, and at the end of a build in `package main`,
we inject a function into the runtime's `internal/abi` package to reverse the obfuscation
for those names which can be used for reflection.
This does mean that the obfuscation for these names is very weak, as the binary
contains a one-to-one mapping to their original names, but they cannot be obfuscated
without breaking too many Go packages out in the wild. There is also some amount
of overhead in `internal/abi` due to this, but we aim to make the overhead insignificant.
Fixes#884, #799, #817, #881, #858, #843, #842Closes#406
This allows me to collect a full CPU profile, showing us that we clearly
spend too much CPU time in garbage collection.
When collecting a full memory profile, we can see where the allocations
come from now:
Showing nodes accounting for 5636770, 31.07% of 18141579 total
Dropped 630 nodes (cum <= 90707)
Showing top 10 nodes out of 278
flat flat% sum% cum cum%
1692229 9.33% 9.33% 1910679 10.53% encoding/gob.decStringSlice
1005596 5.54% 14.87% 1005596 5.54% golang.org/x/tools/go/ssa.buildReferrers
458753 2.53% 17.40% 458753 2.53% go/scanner.(*Scanner).scanIdentifier
458752 2.53% 19.93% 458752 2.53% reflect.(*structType).Field
425984 2.35% 22.28% 448136 2.47% go/parser.(*parser).parseIdent
390049 2.15% 24.43% 390049 2.15% golang.org/x/tools/go/ssa.(*BasicBlock).emit
327683 1.81% 26.23% 371373 2.05% golang.org/x/tools/go/ssa.NewConst
311296 1.72% 27.95% 1024551 5.65% mvdan.cc/garble.(*transformer).transformGoFile.func1
287891 1.59% 29.54% 287891 1.59% encoding/gob.decString
278537 1.54% 31.07% 657043 3.62% golang.org/x/tools/go/ssa.(*builder).compLit
This method can work for any invocation of garble,
but for now we only directly wire it up for `go test -bench`.
It can still be used for regular invocations of `garble build`.
x/exp/rand was being used for no apparent reason; use math/rand.
x/exp/maps and x/exp/slices can be replaced with maps and slices
respectively now that we require Go 1.23 or later.
Note that the APIs are slightly different due to iterators.
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.
Otherwise we miscalculate int sizes, type sizes, alignments, and so on.
Caught by the GOARCH=386 go test on CI, since the os package imports
internal/syscall/unix, which uses arch-dependent padding.
The different padding between our incorrect use of go/types
and the correct typechecking done by the compiler caused different
obfuscation of fields, as the struct types stringified differently,
and they are used as a hash salt for field name obfuscation.
Right now, we only have linker patches for Go 1.22.x.
We only ever maintain those for one or two major Go versions at a time.
If a user tries to use the Go toolchain from 1.21, we already fail
with "Go version too old" messages early on, but we don't for 1.23,
causing a relatively confusing error later on when we link a binary:
cannot get modified linker: cannot retrieve linker patches: open patches/go1.23: file does not exist
Instead, fail early and with a good error message.
Panicking in small helpers or in funcs that don't return error
has proved useful to keep code easier to maintain,
particularly for cases that should typically never happen.
However, in these cases we can error just as easily.
In particular, I was getting a panic whenever I forgot
that I was running garble with Go master (1.23), which is over the top.
We no longer need to worry about the scope of range variables,
we can iterate over integers directly, and we can use cmp.Or too.
I haven't paid close attention to using these everywhere.
This is mainly testing out the new features where I saw some benefit.
That is, the assembly line
#include "foo.h" // bar
would make garble run into an error, as we would try to parse
the #include directive before we stripped comments,
effectively trying to unquote the string
"foo.h" // bar
rather than just the included filename
"foo.h"
Add test cases for asm but also cgo, while at it.
Fixes#812.
TypeName.Pkg is documented as:
Pkg returns the package to which the object belongs.
The result is nil for labels and objects in the Universe scope.
When a struct type embeds a builtin alias type, such as byte,
this would lead to a panic since we assumed we could use the Pkg method.
Fixes#798.
This was a long standing TODO, and a user finally ran into it.
The fix isn't terribly straightforward, but I'm pretty happy with it.
Add a test case where the same struct field is identical
with no tag, with the "tagged1" json tag, and again with "tagged2".
While here, we add a test case for a regular named field too.
Fixes#801.
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.
Otherwise we have to update that `@semver` string
alongside the regular x/tools updates in go.mod.
There's no reason to separate the two versions either.
Up until now, the new SSA reflection detection relied on call sites
to propagate which objects (named types, struct fields) used reflection.
For example, given the code:
json.Marshal(new(T))
we would first record that json.Marshal calls reflect.TypeOf,
and then that the user's code called json.Marshal with the type *T.
However, this would not catch a slight variation on the above:
var t T
reflect.TypeOf(t)
t.foo = struct{bar int}{}
Here, type T's fields such as "foo" and "bar" are not obfuscated,
since our logic sees the call site and marks the type T recursively.
However, the unnamed `struct{bar int}` type was still obfuscated,
causing errors such as:
cannot use struct{uKGvcJvD24 int}{} (value of type struct{uKGvcJvD24 int}) as struct{bar int} value in assignment
The solution is to teach the analysis about *ssa.Store instructions
in a similar way to how it already knows about *ssa.Call instructions.
If we see a store where the destination type is marked for reflection,
then we mark the source type as well, fixing the bug above.
This fixes obfuscating github.com/gogo/protobuf/proto.
A number of other Go modules fail with similar errors,
and they look like very similar bugs,
but this particular fix doesn't apply to them.
Future incremental fixes will try to deal with those extra cases.
Fixes#685.
Update CI to use a newer version of Go master,
now that we're already getting release candidates.
Look at the diffs between Go 1.20 and master of `go help build`
and `go help testflag`, and add two flags that were recently added.
While here, bump a hopeful TODO for a feature request,
since that one definitely did not happen for 1.21.
First, teach scripts/gen-go-std-tables.sh to omit test packages,
since runtime/metrics_test would always result in an error.
Instead, make transformLinkname explicitly skip that package,
leaving a comment about a potential improvement if needed.
Second, the only remaining "not found" error we had was "maps" on 1.20,
so rewrite that check based on ImportPath and GoVersionSemver.
Third, detect packages with the "exclude all Go files" error
by looking at CompiledGoFiles and IgnoredGoFiles, which is less brittle.
This means that we are no longer doing any filtering on pkg.Error.Err,
which means we are less likely to break with Go error message changes.
Fourth, the check on pkg.Incomplete is now obsolete given the above,
meaning that the CompiledGoFiles length check is plenty.
Finally, stop trying to be clever about how we print errors.
Now that we're no longer skipping packages based on pkg.Error values,
printing pkg.DepsErrors was causing duplicate messages in the output.
Simply print pkg.Error values with only minimal tweaks:
including the position if there is any, and avoiding double newlines.
Overall, this makes our logic a lot less complicated,
and garble still works the way we want it to.
computeLinkerVariableStrinsg had an unusedargument.
Only skip obfuscating the name "FS" in the "embed" package.
The reflect methods no longer use the transformer receiver type,
so that TODO now feels unnecessary. Many methods need to be aware
of what the current types.Package is, and that seems reasonable.
We no longer use writeFileExclusive for our own cache on disk,
so the TODO about using locking or atomic writes is no longer relevant.
Our cache is now robust against different Go package build inputs
which result in exactly the same build outputs.
In the past, this caused Go's cache to be filled but not ours.
In the present, our cache is just as resilient as Go's is.
This means we now have a unified cache directory for garble,
which is now documented in the README.
I considered using the same hash-based cache used for pkgCache,
but decided against it since that cache implementation only stores
regular files without any executable bits set.
We could force the cache package to do what we want here,
but I'm leaning against it for now given that single files work OK.
Some users had been running into "cannot load cache entry" errors,
which could happen if garble's cache files in GOCACHE were removed
when Go's own cache files were not.
Now that we've moved to our own separate cache directory,
and that we've refactored the codebase to depend less on globals
and no longer assume that we're loading info for the current package,
we can now compute a pkgCache entry for a dependency if needed.
We add a pkgCache.CopyFrom method to be able to append map entries
from one pkgCache to another without needing an encoding/gob roundtrip.
We also add a parseFiles helper, since we now have three bits of code
which need to parse a list of Go files from disk.
Fixes#708.
loadPkgCache also uses different pkgCache variables
depending on whether it finds a direct cache hit or not.
Now we only initialize an entirely new pkgCache
with the first two ReflectAPIs entries when we get a direct cache miss,
since a direct cache hit will already load those from the cache.
It is true that each garble process only obfuscates up to one package,
which is why we made them globals to begin with.
However, garble does quite a lot more now,
such as reversing the obfuscation of many packages at once.
Having a global "current package" variable makes mistakes easier.
Some funcs, like those in transformFuncs, are now transformer methods.
We've been obfuscating all linknamed names for a while now,
so the part in the docs about "recording" is no longer true.
All it does is transform the directives to use obfuscated names.
Give it a better name and rewrite the docs.
The name and docs on that func were wildly out of date,
since it no longer has anything to do with reflection at all.
We only use the linkerVariableStrings map with -literals,
so we can avoid the call entirely if the flag isn't set.
Neither of them has anything to do with transforming Go code;
they simply load or compute the information necessary for doing so.
Split typecheck into two functions as well.
The new typecheck function only does typechecking and nothing else.
A new comptueFieldToStruct func fills the fieldToStruct map,
which depends on typecheck, but is not needed when computing pkgCache.
This isolation also forces us to separate the code that fills pkgCache
from the code that fills the in-memory-only maps in transformer,
removing the need for the NOTE that we had left around as a reminder.
That is, stop reusing "transformer" as the receiver on methods,
and stop writing the results to the global curPkgCache struct.
Soon we will need to support computing pkgCache for any dependency,
not just the current package, to make the caching properly robust.
This allows us to fill reflectInspector with different values.
The explicit isolation also helps prevent bugs.
For instance, we were calling recursivelyRecordAsNotObfuscated from
transformCompile, which happens after we have loaded or saved pkgCache.
Meaning, the current package sees a larger pkgCache than its dependents.
In this particular case it wasn't causing any bugs,
since the two reflect types in question only had unexported fields,
but it's still good to treat pkgCache as read-only in transformCompile.
Daniel Martí
Paul Scheduikat
Jamison Lahman
NHAS
Hritik Vijay
pagran