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.
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.
We were recently altering the logic in reflect.go for type names,
which could have broken this kind of valid use of reflection.
Add a regression test, which I verified would break before my last
change to "simplify" the logic, which actually changed the logic,
as xuannv112 correctly pointed out.
After thinking about the change in behavior for a little while,
I realised that the new behavior is more correct, hence the test.
A recent PR added a bigger regression test for go-spew,
and fixed an issue where we would obfuscate local named types
even if they were embedded into local structs used for reflection.
This would effectively mean we were obfuscating one field name,
the one derived from the embedding, which we didn't want to.
The fix did this by searching for embedded objects with extra code.
However, as far as I can tell, that isn't necessary;
we can do the right thing by recording all local type names
just like we already do for all field names.
This results in less complicated code, and avoids needing special logic
to handle embedding struct types, so I reckon it's a win.
Add even more tests to convince myself that we're still obfuscating
local types and field names which aren't used for reflection.
See the added comment and test, which failed before the fix when
the encoded certificate was decoded.
It took a while to find the culprit in asn1, but in hindsight it's easy:
case reflect.Slice:
if t.Elem().Kind() == reflect.Uint8 {
return false, TagOctetString, false, true
}
if strings.HasSuffix(t.Name(), "SET") {
return false, TagSet, true, true
}
return false, TagSequence, true, true
Fixes#711.
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.
It assumes that reflect.Value has a field named "flag",
which wasn't the case with obfuscated builds as we obfuscated it.
We already treated the reflect package as special,
for instance when not obfuscating Method or MethodByName.
In a similar fashion, mark reflect's rtype and Value types to not be
obfuscated alongside their field names. Note that rtype is the
implementation behind the reflect.Type interface.
This fix is fairly manual and repetitive.
transformCompile, transformLinkname, and transformAsm should all
use the same mechanism to tell if names should be obfuscated.
However, they do not do that right now, and that refactor feels too
risky for a bugfix release. We add more TODOs instead.
We're not adding go-spew to scripts/check-third-party.sh since the
project is largely abandoned. It's not even a Go module yet.
The only broken bit from it is what we've added to our tests.
Fixes#676.
We were obfuscating reflect's package path and its declared names,
but the toolchain wants to detect the presence of method reflection
to turn down the aggressiveness of dead code elimination.
Given that the obfuscation broke the detection,
we could easily end up in crashes when making reflect calls:
fatal error: unreachable method called. linker bug?
goroutine 1 [running]:
runtime.throw({0x50c9b3?, 0x2?})
runtime/panic.go:1047 +0x5d fp=0xc000063660 sp=0xc000063630 pc=0x43245d
runtime.unreachableMethod()
runtime/iface.go:532 +0x25 fp=0xc000063680 sp=0xc000063660 pc=0x40a845
runtime.call16(0xc00010a360, 0xc00000e0a8, 0x0, 0x0, 0x0, 0x8, 0xc000063bb0)
runtime/wcS9OpRFL:728 +0x49 fp=0xc0000636a0 sp=0xc000063680 pc=0x45eae9
runtime.reflectcall(0xc00001c120?, 0x1?, 0x1?, 0x18110?, 0xc0?, 0x1?, 0x1?)
<autogenerated>:1 +0x3c fp=0xc0000636e0 sp=0xc0000636a0 pc=0x462e9c
Avoid obfuscating the three names which cause problems: "reflect",
"Method", and "MethodByName".
While here, we also teach obfuscatedImportPath to skip "runtime",
as I also saw that the toolchain detects it for many reasons.
That wasn't a problem yet, as we do not obfuscate the runtime,
but it was likely going to become a problem in the future.
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í
Paul Scheduikat
xuannv