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

Export symbols used in inline functions #2407

Merged
merged 1 commit into from Dec 7, 2016

Conversation

jbrianceau
Copy link
Contributor

fixed_address_empty_string and default_instance symbols
are used in inline functions. We have to export them properly to
avoid undefined reference link errors.

@grpc-kokoro
Copy link

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

1 similar comment
@grpc-kokoro
Copy link

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

@jbrianceau
Copy link
Contributor Author

Mmm, the change works well in Chromium, but it doesn't seem to work very well here :/

@jbrianceau
Copy link
Contributor Author

jbrianceau commented Nov 23, 2016

It's better after having regenerated descriptor proto, the windows build is still broken though.

Maybe I should deinline functions instead of trying to export symbol ? What do you think ?

@xfxyjwf xfxyjwf self-assigned this Nov 23, 2016
@xfxyjwf
Copy link
Contributor

xfxyjwf commented Nov 23, 2016

deinline is usually not an acceptable solution because of its performance impact. Our appveyor already covers DLL build and it's passing without this pull request. Could it be a build setup problem of Chrome?

@jbrianceau
Copy link
Contributor Author

deinline is usually not an acceptable solution because of its performance impact.

Ok.

Our appveyor already covers DLL build and it's passing without this pull request. Could it be a build setup problem of Chrome?

That's what I do not understand yet : why Windows version builds fine without my patch and fails with it when we use protobuf as a shared library ? (whereas Linux is fine).

For instance :

  • we have in src/google/protobuf/generated_message_util.h :
extern ExplicitlyConstructed< ::std::string> fixed_address_empty_string;

LIBPROTOBUF_EXPORT inline const ::std::string& GetEmptyStringAlreadyInited() {
  return fixed_address_empty_string.get();
}

  • and in src/google/protobuf/generated_message_util.cc :
ExplicitlyConstructed< ::std::string> fixed_address_empty_string;

So when protobuf is a shared library, the symbol google::protobuf::internal::fixed_address_empty_string is hidden in it :

   115: 00000000000370a0    16 OBJECT  LOCAL  HIDDEN    27 _ZN6google8protobuf8internal26fixed_address_empty_stringE

So when a piece of code from another module includes generated_message_util.h and links with protobuf shared library, there is an undefined reference to google::protobuf::internal::fixed_address_empty_string as the symbol is hidden.

Am I missing something here ?

@thirdknife
Copy link

I am unable to build version 3.1.0 on windows with MSVS CE 2015. Getting an unresolved external symbol on:

LNK2001 unresolved external symbol "class google::protobuf::internal::ExplicitlyConstructed<class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > > google::protobuf::internal::fixed_address_empty_string" (?fixed_address_empty_string@internal@protobuf@google@@3V?$ExplicitlyConstructed@V?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@@123@A)

and

LNK2001 unresolved external symbol "__int64 google::protobuf::internal::empty_string_once_init_" (?empty_string_once_init_@internal@protobuf@google@@3_JA)

Any hints where I can look into to fix the source?

@jbrianceau
Copy link
Contributor Author

Does this pull request fix your issue @thirdknife ? Especially this commit : 8a07630

@thirdknife
Copy link

This fails the protobuf build with the following error:

google::protobuf::internal::fixed_address_empty_string': redefinition; different storage class libprotobuf

fixed_address_empty_string symbol is used in an inline function.
We have to export it to avoid undefined reference link errors.
@jbrianceau
Copy link
Contributor Author

I made the mistake of exporting the symbol definition in the source file instead of exporting its declaration in the header file.
@xfxyjwf, I have rebased the change and hopefully fixed the issue.

@xfxyjwf xfxyjwf merged commit 850d573 into protocolbuffers:master Dec 7, 2016
@jbrianceau
Copy link
Contributor Author

Follow-up : #2462

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

Successfully merging this pull request may close these issues.

None yet

5 participants