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

Add support for symbol visibility export files #4747

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

jpakkane
Copy link
Member

@jpakkane jpakkane commented Jan 8, 2019

Not finished yet!

Documentation missing. Possibly other stuff missing. Open questions:

  • should this be called symbol_export_files instead since it can have several
  • should we limit it to at most one file
  • how should this tie in with free form linker scripts
  • are linker scripts a thing outside of GNU/Clang

@emersion
Copy link
Contributor

emersion commented Jan 9, 2019

This is neat, I'm still worried about using the same name for a file that is different on each platform.

Clang supports -exported_symbols_list on all platforms it seems.

how should this tie in with free form linker scripts

version script != linker script

are linker scripts a thing outside of GNU/Clang

About version scripts, I don't think macOS/Windows supports symbol versioning.

@jpakkane
Copy link
Member Author

This is neat, I'm still worried about using the same name for a file that is different on each platform.

This is is a bit problematic but the alternative is to have a kwarg per platform (and compiler). That causes kwarg explosion and does not really work either.

Clang supports -exported_symbols_list on all platforms it seems.

I would hazard a guess that -exported_symbols_list would only be used on Mac platforms even if Clang supported it everywhere. At least on Linux you'd want to use the classical approach so one file would work both with GCC and Clang.

version script != linker script

Yes. The point was what should we do if both of those are defined. Or possibly if we should use the same code to handle both as they are semantically similar.

if cc.get_id() == 'msvc'
suffix = '.sym'
else
suffix = host_machine.system() == 'darwin' ? '-clang.map' : '.map'
Copy link
Contributor

Choose a reason for hiding this comment

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

meson.build:14:0: ERROR:  Nonexisting file bob-clang.map.

@@ -1156,6 +1156,10 @@ def get_linker_always_args(self):
return basic + ['-Wl,-headerpad_max_install_names']
return basic

def get_symbol_export_file_args(self, path):
if self.compiler_type.is_osx_compiler:
Copy link
Contributor

Choose a reason for hiding this comment

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

Does GCC support -exported_symbols_list on macOS? (Be careful, GCC on Linux interprets -exported_symbols_list as the -e flag with the value xported_symbols_list)

@emersion
Copy link
Contributor

One thing I'm still worried about is that the user needs to implement the exact same "flag matching logic" than Meson in his meson.build file. The user needs to guess which flag Meson will use to feed the proper file. For instance, if building on Linux with Clang, -exported_symbols_list should not be used even if it's supported because it'll get used as a version script.

Maybe a more fail-safe API would be something among the lines of:

shared_library(symbol_export_file: {
  'version_script': '',
  'msvc': '',
  'macos': '', # This one seems to be macOS-specific because all symbols seem to need a leading underscore
})

@@ -1067,6 +1067,9 @@ def gen_vcxproj(self, target, ofname, guid):
extra_link_args += compiler.get_option_link_args(self.environment.coredata.compiler_options)
(additional_libpaths, additional_links, extra_link_args) = self.split_link_args(extra_link_args.to_native())

unquoted_args, _ = self.get_export_symbol_file_args(compiler, target, absolute_paths=True)
Copy link
Member

Choose a reason for hiding this comment

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

[Flake8] [E222] multiple spaces after operator

posted by Sider

args += linker.get_symbol_export_file_args(final_path)
dep_files.append(relpath)
return (args, dep_files)

Copy link
Member

Choose a reason for hiding this comment

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

[Flake8] [W391] blank line at end of file

posted by Sider

@jpakkane
Copy link
Member Author

Finally passes on all platforms. Now we need to make the actual decisions on whether to include this or not (and in what form).

@jpakkane
Copy link
Member Author

Maybe a more fail-safe API would be something among the lines of:

You can replicate that already with something like this:

symbol_files = {'windows': 'foo.sym',
     ...
   }

shared_library(..., version_script: symbol_files[host_machine.system()])

@emersion
Copy link
Contributor

This won't work, because GCC on macOS will use --version-script and Clang on macOS will use -exported-symbols-list.

@jpakkane
Copy link
Member Author

But those two files have different syntax, right? They would need to point to different files then. So in order to make it work, we'd need to parametrize the kwarg both on the compiler and the platform (and, because everything is terrible, possibly other things as well). That's a fairly big state space explosion.

@emersion
Copy link
Contributor

The version script should work with GCC on all platforms.

@jpakkane
Copy link
Member Author

Well if the scripts are unique to a compiler, then the same applies as above, just use the compiler id as the key. If they are not, and you need to specify both a compiler and a platform to know which file to use then you need to do manual work anyway.

@emersion
Copy link
Contributor

emersion commented Jan 28, 2019

Point is, Meson does the complex matching already, why duplicate the work? Also what happens if Meson's matching changes for a specific OS/compiler combination?

@dcbaker
Copy link
Member

dcbaker commented Feb 13, 2019

So, odd question. Have we considered implementing one format (any of the existing formats) and having meson automatically create rules to convert said format if necessary? ie, meson always uses a gcc style visibility script, and we convert that to a mac symbol visibility or msvc symbol visibility script at configure or build time? or are there dragons there I'm not thinking of?

It would be really nice to not have to define 5 different files to do the same thing (or play the awful #define game)

@jpakkane
Copy link
Member Author

Thing I can think of:

  • the contents are very different, many GNU symbol scripts do versioning, which the other formats can't do (that I know of)
  • different platforms may export a different set of symbols so the format would need to support that

The "correct" solution is to use visibility macros rather than linker symbol scripts for everything except GNU style sumbol versioning. That requires code changes, though, and people seem to resist doing that due to various reasons.

@bonzini
Copy link
Contributor

bonzini commented Feb 27, 2019

The "correct" solution is to use visibility macros rather than linker symbol scripts for everything except GNU style sumbol versioning. That requires code changes, though, and people seem to resist doing that due to various reasons.

FWIW, libtool used a simple list file and automatically generated a version script when needed. See -export-symbols here.

@jpakkane
Copy link
Member Author

Libtool does not support the Visual Studio linker and it is unknown whether it supports lld either.

@bonzini
Copy link
Contributor

bonzini commented Feb 28, 2019

Libtool does not support the Visual Studio linker and it is unknown whether it supports lld either

I was just remarking on the interface of giving a simple list of exported symbols. Such a list can be converted to a DEF file for LINK.EXE, and can be passed to -exported_symbols_list on macOS. Ut also covers lld because it supports DEF files on Windows and -exported_symbols_list on Mac.

@emersion
Copy link
Contributor

can be passed to -exported_symbols_list on macOS

Note: on macOS symbols need to be prefixed with an underscore. See e.g. https://github.com/emersion/mrsh/blob/master/libmrsh.darwin.sym

@jpakkane
Copy link
Member Author

jpakkane commented Mar 3, 2019

Just realized that there is already vs_module_defs. This should work together with that in some way, methinks. Currently it does not.

@ePirat
Copy link
Contributor

ePirat commented Jun 5, 2019

The "correct" solution is to use visibility macros rather than linker symbol scripts for everything except GNU style sumbol versioning. That requires code changes, though, and people seem to resist doing that due to various reasons.

This is not possible in all cases, for example you might use nasm or some other compiler that does not support any such annotations so there are cases where you need to limit symbols visibility when linking the final library.

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

Successfully merging this pull request may close these issues.

6 participants