addons: Missing symbol for Abseil ParseTime Op
Describe the bug
Since the 20101030 nightly, our library fails when opening the _parse_time_op.so because of an undefined symbol error. Typically this occurs when there is an ABI incompatibility between core TensorFlow and our custom-ops.
tensorflow.python.framework.errors_impl.NotFoundError: .../custom_ops/text/_parse_time_op.so: undefined symbol: _ZN4absl9ParseTimeERKSsS1_PNS_4TimeEPSs
This particular custom-op utilizes absl and the mangled symbol name does not match what our compiled operation thinks it should be. I’m not really sure how this would happen since the rest of the TensorFlow library was compiled with a compatible gcc version.
CC’ing some people who may have insight into how this can happen: @yifeif @r4nt @perfinion @gunan
I suppose we could make absl a dependency in our TFA build and compile it ourselves but this seems to be going down a bad path.
About this issue
- Original URL
- State: closed
- Created 5 years ago
- Comments: 20 (12 by maintainers)
Commits related to this issue
- Patch @com_google_absl to always export [time](https://github.com/abseil/abseil-cpp/blob/ab3552a18964e7063c8324f45b3896a6a20b08a8/absl/time/BUILD.bazel#L30) symbols. This should fix https://github.co... — committed to scentini/tensorflow by scentini 5 years ago
- Patch @com_google_absl to always export //absl/time:time symbols. This should fix tensorflow/addons#663 — committed to scentini/tensorflow by scentini 5 years ago
- Patch @com_google_absl to always export //absl/time:time symbols. This should fix tensorflow/addons#663 (cherry picked from commit 17a23945abe427cf2fe6677d26e65558109f6f10) — committed to timokau/tensorflow by scentini 5 years ago
- PR #34044: Patch @com_google_absl to always export //absl/time:time symbols Imported from GitHub PR https://github.com/tensorflow/tensorflow/pull/34044 This should fix tensorflow/addons#663, caused ... — committed to tensorflow/tensorflow by tensorflower-gardener 5 years ago
- Set --incompatible_remove_legacy_whole_archive to false for TF. Test coverage proved insufficient to catch errors caused by setting the default to true. This PR should fix https://github.com/tensorf... — committed to tensorflow/tensorflow by tensorflower-gardener 5 years ago
- Automated g4 rollback of changelist 279340363. *** Reason for rollback *** This cl breaks Windows build with --noincompatible_remove_legacy_whole_archive, which is needed in order to fix https://gith... — committed to tensorflow/tensorflow by tensorflower-gardener 5 years ago
After discussion with @r4nt, it might be better to not pass the absl dependency to _parse_time_op.so, but rather add
alwayslink = 1to theabsllibrary. (thus make sure that absl symbols are present in @local_config_tf//:libtensorflow_framework)The reason for this is, if we pass
abslas a dependency, we may end up linking global initializers multiple times.I can send out a fix for this tomorrow.
@seanpmorgan it appears to work now with
Unfortunately this fix caused internal breakages, so we’ll be rolling it back. Whatever the solution is, it will be cherrypicked onto r2.1. I’ll keep you updated.
The PR that fixed this was reverted in 5acf6bd. An internal PR that’ll fix this and similar issues by reverting the change that introduced these errors is in the process of being merged.
Yep, seems like it is related to f58c9f8, which changes what we export, see https://github.com/bazelbuild/bazel/issues/7362.
The absl symbols probably came from here, and due to f58c9f8 we just don’t export the symbol anymore. I think the right way forward is to make _parse_time_op.so depend on absl.
@scentini for whether this might be due to us not exporting all symbols from absl any more