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.

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

Fix to allow AOT compilers to play nicely with reflection #4559

Merged
merged 1 commit into from
Apr 27, 2018

Conversation

jskeet
Copy link
Contributor

@jskeet jskeet commented Apr 26, 2018

With this fix, Unity using IL2CPP should work with one of two
approaches:

  • Call FileDescriptor.ForceReflectionInitialization<T> for every
    enum present in generated code (including oneof case enums)
  • Ensure that IL2CPP uses the same code for int and any int-based
    enums

The former approach is likely to be simpler, unless IL2CPP changes
its default behavior. We could potentially generate the code
automatically, but that makes me slightly uncomfortable in terms of
generating code that's only relevant in one specific scenario. It
would be reasonably easy to write a tool (separate from protoc) to
generate the code required for any specific set of assemblies, so
that Unity users can include it in their application. We can always
decide to change to generate it automatically later.

With this fix, Unity using IL2CPP should work with one of two
approaches:

- Call `FileDescriptor.ForceReflectionInitialization<T>` for every
  enum present in generated code (including oneof case enums)
- Ensure that IL2CPP uses the same code for int and any int-based
  enums

The former approach is likely to be simpler, unless IL2CPP changes
its default behavior. We *could* potentially generate the code
automatically, but that makes me slightly uncomfortable in terms of
generating code that's only relevant in one specific scenario. It
would be reasonably easy to write a tool (separate from protoc) to
generate the code required for any specific set of assemblies, so
that Unity users can include it in their application. We can always
decide to change to generate it automatically later.
@jskeet jskeet force-pushed the reflection-forcing branch from 7d5ba37 to b2639b2 Compare April 26, 2018 15:41
Copy link
Contributor

@jtattermusch jtattermusch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@jskeet jskeet merged commit 1b219a1 into protocolbuffers:master Apr 27, 2018
@leohahn
Copy link

leohahn commented Dec 3, 2018

For me this fix did not work. With the following proto file:

syntax = "proto3";

package protos;

message Error {
  string code = 1;
  string msg = 2;
  map<string, string> metadata = 3;
}

And adding

FileDescriptor.ForceReflectionInitialization<Protos.Error>();

to a static constructor on a class, it still raises a SIGABRT on compiled code.

@jskeet
Copy link
Contributor Author

jskeet commented Dec 3, 2018

The fix is for enums - that's not an enum. Did you call ForceReflectionInitialization with all the enums in your proto?

@jskeet jskeet deleted the reflection-forcing branch December 3, 2018 22:25
@jskeet
Copy link
Contributor Author

jskeet commented Dec 3, 2018

@leohahn: (Now I'm not on mobile...) did you get any errors logged before the crash? I seem to remember that when I tested it, I got a useful error message showing what was missing. That may only be in some environments though.

What were you doing that crashed it - just printing out the string representation of a proto, or something else? I'd like to reproduce this if I can.

@leohahn
Copy link

leohahn commented Dec 4, 2018

Oh my bad, yeah the proto has no enums. This is a simple project that shows the error, you can see the code in Assets/Test.cs.

https://github.com/leohahn/error-libpitaya-protobuf

The code works in the editor but when compiling for iOS it crashes. Here's the stacktrace:

DescriptorValidationException: google.protobuf.Value.kind: Method ClearKind not found in Google.Protobuf.WellKnownTypes.Value
  at Google.Protobuf.Reflection.OneofDescriptor.CreateAccessor (System.String clrName) [0x00000] in <00000000000000000000000000000000>:0 
  at Google.Protobuf.Reflection.OneofDescriptor..ctor (Google.Protobuf.Reflection.OneofDescriptorProto proto, Google.Protobuf.Reflection.FileDescriptor file, Google.Protobuf.Reflection.MessageDescriptor parent, System.Int32 index, System.String clrName) [0x00000] in <00000000000000000000000000000000>:0 
  at Google.Protobuf.Reflection.MessageDescriptor+<>c__DisplayClass4_0.<.ctor>b__0 (Google.Protobuf.Reflection.OneofDescriptorProto oneof, System.Int32 index) [0x00000] in <00000000000000000000000000000000>:0 
  at Google.Protobuf.Reflection.DescriptorUtil.ConvertAndMakeReadOnly[TInput,TOutput] (System.Collections.Generic.IList`1[T] input, Google.Protobuf.Reflection.DescriptorUtil+IndexedConverter`2[TInput,TOutput] converter) [0x00000] in <00000000000000000000000000000000>:0 
  at Google.Protobuf.Reflection.MessageDescriptor..ctor (Google.Protobuf.Reflection.DescriptorProto proto, Google.Protobuf.Reflection.FileDescriptor file, Google.Protobuf.Reflection.MessageDescriptor parent, System.Int32 typeIndex, Google.Protobuf.Reflection.GeneratedClrTypeInfo generatedCodeInfo) [0x00000] in <00000000000000000000000000000000>:0 
  at Google.Protobuf.Reflection.FileDescriptor+<>c__DisplayClass1_0.<.ctor>b__0 (Google.Protobuf.Reflection.DescriptorProto message, System.Int32 index) [0x00000] in <00000000000000000000000000000000>:0 
  at Google.Protobuf.Reflection.DescriptorUtil.ConvertAndMakeReadOnly[TInput,TOutput] (System.Collections.Generic.IList`1[T] input, Google.Protobuf.Reflection.DescriptorUtil+IndexedConverter`2[TInput,TOutput] converter) [0x00000] in <00000000000000000000000000000000>:0 
  at Google.Protobuf.Reflection.FileDescriptor..ctor (Google.Protobuf.ByteString descriptorData, Google.Protobuf.Reflection.FileDescriptorProto proto, Google.Protobuf.Reflection.FileDescriptor[] dependencies, Google.Protobuf.Reflection.DescriptorPool pool, System.Boolean allowUnknownDependencies, Google.Protobuf.Reflection.GeneratedClrTypeInfo generatedCodeInfo) [0x00000] in <00000000000000000000000000000000>:0 
  at Google.Protobuf.Reflection.FileDescriptor.BuildFrom (Google.Protobuf.ByteString descriptorData, Google.Protobuf.Reflection.FileDescriptorProto proto, Google.Protobuf.Reflection.FileDescriptor[] dependencies, System.Boolean allowUnknownDependencies, Google.Protobuf.Reflection.GeneratedClrTypeInfo generatedCodeInfo) [0x00000] in <00000000000000000000000000000000>:0 
  at Google.Protobuf.Reflection.FileDescriptor.FromGeneratedCode (System.Byte[] descriptorData, Google.Protobuf.Reflection.FileDescriptor[] dependencies, Google.Protobuf.Reflection.GeneratedClrTypeInfo generatedCodeInfo) [0x00000] in <00000000000000000000000000000000>:0 
  at Google.Protobuf.WellKnownTypes.StructReflection..cctor () [0x00000] in <00000000000000000000000000000000>:0 
  at Google.Protobuf.WellKnownTypes.Value.get_Descriptor () [0x00000] in <00000000000000000000000000000000>:0 
  at Google.Protobuf.JsonParser..cctor () [0x00000] in <00000000000000000000000000000000>:0 
  at Test.Start () [0x00000] in <00000000000000000000000000000000>:0 
Rethrow as ArgumentException: Invalid embedded descriptor for "google/protobuf/struct.proto".
  at Google.Protobuf.Reflection.FileDescriptor.FromGeneratedCode (System.Byte[] descriptorData, Google.Protobuf.Reflection.FileDescriptor[] dependencies, Google.Protobuf.Reflection.GeneratedClrTypeInfo generatedCodeInfo) [0x00000] in <00000000000000000000000000000000>:0 
  at Google.Protobuf.WellKnownTypes.StructReflection..cctor () [0x00000] in <00000000000000000000000000000000>:0 
  at Google.Protobuf.WellKnownTypes.Value.get_Descriptor () [0x00000] in <00000000000000000000000000000000>:0 
  at Google.Protobuf.JsonParser..cctor () [0x00000] in <00000000000000000000000000000000>:0 
  at Test.Start () [0x00000] in <00000000000000000000000000000000>:0 
Rethrow as TypeInitializationException: The type initializer for 'Google.Protobuf.WellKnownTypes.StructReflection' threw an exception.
  at Google.Protobuf.WellKnownTypes.Value.get_Descriptor () [0x00000] in <00000000000000000000000000000000>:0 
  at Google.Protobuf.JsonParser..cctor () [0x00000] in <00000000000000000000000000000000>:0 
  at Test.Start () [0x00000] in <00000000000000000000000000000000>:0 
Rethrow as TypeInitializationException: The type initializer for 'Google.Protobuf.JsonParser' threw an exception.
  at Test.Start () [0x00000] in <00000000000000000000000000000000>:0 

@jskeet
Copy link
Contributor Author

jskeet commented Dec 4, 2018

Hmm, interesting. Okay, it looks like I'll need to take a deeper look into that. To be honest, it's unlikely that I'll be able to get into that before the end of the year, but I'll see what I can do.

Could you file a new issue with all these details, along with version information etc? I'll then assign it to myself and get to it when I can.

@jskeet
Copy link
Contributor Author

jskeet commented Dec 4, 2018

As a workaround in the meantime, it's possible that you could add code to fix this yourself for now. Try adding this to your project:

global::Google.Protobuf.WellKnownTypes.Value value = new Value();
value.ClearKind();

And see if that at least changes the error message. If it does - particularly if it changes it to another ClearXxx method somewhere else - that'll make it easier to find. Applying that workaround lots of times (possibly for every oneof) may well fix it, too.

@jskeet
Copy link
Contributor Author

jskeet commented Dec 4, 2018

Hmm.... il2cpp is no longer working for me running my previous tests. It may take a little longer than expected to test this...

@leohahn
Copy link

leohahn commented Dec 4, 2018

Hey, thanks for the help. I'll open another issue. I'll try the workaround as well.

@leohahn
Copy link

leohahn commented Dec 4, 2018

The issue is here: #5422

@jskeet
Copy link
Contributor Author

jskeet commented Dec 4, 2018

@leohahn: Thanks. I've sorted out my il2cpp issue now, and I'm trying to reproduce your specific example.

@leohahn
Copy link

leohahn commented Dec 4, 2018

Cool, adding your workaround returns me this error:

DescriptorValidationException: google.protobuf.ListValue.values: Property Values not found in Google.Protobuf.WellKnownTypes.ListValue
  at Google.Protobuf.Reflection.FieldDescriptor.CreateAccessor () [0x00000] in <00000000000000000000000000000000>:0 
  at Google.Protobuf.Reflection.FieldDescriptor.CrossLink () [0x00000] in <00000000000000000000000000000000>:0 
  at Google.Protobuf.Reflection.MessageDescriptor.CrossLink () [0x00000] in <00000000000000000000000000000000>:0 
  at Google.Protobuf.Reflection.FileDescriptor.CrossLink () [0x00000] in <00000000000000000000000000000000>:0 
  at Google.Protobuf.Reflection.FileDescriptor.BuildFrom (Google.Protobuf.ByteString descriptorData, Google.Protobuf.Reflection.FileDescriptorProto proto, Google.Protobuf.Reflection.FileDescriptor[] dependencies, System.Boolean allowUnknownDependencies, Google.Protobuf.Reflection.GeneratedClrTypeInfo generatedCodeInfo) [0x00000] in <00000000000000000000000000000000>:0 
  at Google.Protobuf.Reflection.FileDescriptor.FromGeneratedCode (System.Byte[] descriptorData, Google.Protobuf.Reflection.FileDescriptor[] dependencies, Google.Protobuf.Reflection.GeneratedClrTypeInfo generatedCodeInfo) [0x00000] in <00000000000000000000000000000000>:0 
  at Google.Protobuf.WellKnownTypes.StructReflection..cctor () [0x00000] in <00000000000000000000000000000000>:0 
  at Google.Protobuf.WellKnownTypes.Value.get_Descriptor () [0x00000] in <00000000000000000000000000000000>:0 
  at Google.Protobuf.JsonParser..cctor () [0x00000] in <00000000000000000000000000000000>:0 
  at Test.Start () [0x00000] in <00000000000000000000000000000000>:0 
Rethrow as ArgumentException: Invalid embedded descriptor for "google/protobuf/struct.proto".
  at Google.Protobuf.Reflection.FileDescriptor.FromGeneratedCode (System.Byte[] descriptorData, Google.Protobuf.Reflection.FileDescriptor[] dependencies, Google.Protobuf.Reflection.GeneratedClrTypeInfo generatedCodeInfo) [0x00000] in <00000000000000000000000000000000>:0 
  at Google.Protobuf.WellKnownTypes.StructReflection..cctor () [0x00000] in <00000000000000000000000000000000>:0 
  at Google.Protobuf.WellKnownTypes.Value.get_Descriptor () [0x00000] in <00000000000000000000000000000000>:0 
  at Google.Protobuf.JsonParser..cctor () [0x00000] in <00000000000000000000000000000000>:0 
  at Test.Start () [0x00000] in <00000000000000000000000000000000>:0 
Rethrow as TypeInitializationException: The type initializer for 'Google.Protobuf.WellKnownTypes.StructReflection' threw an exception.
  at Google.Protobuf.WellKnownTypes.Value.get_Descriptor () [0x00000] in <00000000000000000000000000000000>:0 
  at Google.Protobuf.JsonParser..cctor () [0x00000] in <00000000000000000000000000000000>:0 
  at Test.Start () [0x00000] in <00000000000000000000000000000000>:0 
Rethrow as TypeInitializationException: The type initializer for 'Google.Protobuf.JsonParser' threw an exception.
  at Test.Start () [0x00000] in <00000000000000000000000000000000>:0 
 
(Filename: currently not available on il2cpp Line: -1)

So there is no Clear anymore

@jskeet
Copy link
Contributor Author

jskeet commented Dec 4, 2018

Aha. It sounds like il2cpp may be being very aggressive with its pruning. One option would be add a use of ListValue.Values from your code, etc - but that's going to be really tedious for you. I don't know why you're seeing that but I'm not, unless that's an il2cpp change.

@leohahn
Copy link

leohahn commented Dec 4, 2018

Well I got a very interesting behaviour. If I make the error happen two times, the second time it works, but the first it throws the exception.

@jskeet
Copy link
Contributor Author

jskeet commented Dec 4, 2018

Wow. Okay, I've no idea about that. It might be worth asking Unity support about that...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants