[conan-center] Forbid linking against removed glibc finite math functions

[valgur]

Adds a linter check for any __*_finite symbols in .a, .so and executable files on Linux to avoid distributing binaries that link against glibc symbols that are no longer available on newer systems with glibc v2.31+. This affects quite a few audio and image libraries that have --ffast-math set, such as mpg123, openjpeg, libx264 and libx265.

See conan-io/conan-center-index#18951 (comment) for context.

Here are also some code snippets to find any affected libraries in a local Conan cache: https://gist.github.com/valgur/d3b1576de71d29467e2cd2434e19af02
And a full list of glibc symbols this check is supposed to catch: https://gist.github.com/valgur/dac1a98174df755cfa85a04fcc5a93a1

[valgur]

Here's a Dockerfile to verify that the new hook catches the issue on Ubuntu 18.04 / glibc 2.27:

FROM conanio/gcc8:1.60.2

USER root
RUN git clone https://github.com/conan-io/conan-center-index.git --depth 1
RUN conan profile new default --detect && conan profile update settings.compiler.libcxx=libstdc++11 default
RUN mkdir -p ~/.conan/hooks/ && \
    wget https://raw.githubusercontent.com/valgur/conan-hooks/feature/forbid-glibc-finite-math-functions/hooks/conan-center.py -O ~/.conan/hooks/conan-center.py && \
    chmod +x ~/.conan/hooks/conan-center.py
RUN conan config set hooks.conan-center
RUN conan create conan-center-index/recipes/mpg123/all /1.31.2@ --build=mpg123 -pr:b=default
RUN objdump -t ~/.conan/data/mpg123/1.31.2/_/_/package/*/lib/libmpg123.a | grep _finite || true

# Uncomment to verify that building with the offending optimization turned off fixes the issue
# RUN CFLAGS=-fno-finite-math-only conan create conan-center-index/recipes/mpg123/all /1.31.2@ --build=mpg123 -pr:b=default
# RUN objdump -t ~/.conan/data/mpg123/1.31.2/_/_/package/*/lib/libmpg123.a | grep _finite || true

Which produces:

Step 7/10 : RUN conan create conan-center-index/recipes/mpg123/all /1.31.2@ --build=mpg123 -pr:b=default
[...]
mpg123/1.31.2 (test package): Running test()
WARN: Remotes registry file missing, creating default one in /root/.conan/remotes.json
[HOOK - conan-center.py] post_package(): ERROR: [USING GLIBC FINITE MATH FUNCTIONS (KB-H078)] Package contains files that link against functions removed from glibc >= v2.31:
bin/mpg123-id3dump: __exp2_finite, __exp_finite, __log_finite
bin/mpg123-strip: __exp2_finite, __exp_finite, __log_finite
bin/mpg123: __exp2_finite, __exp_finite, __log10_finite, __log_finite
bin/out123: __exp_finite, __log10_finite, __log_finite
lib/libmpg123.a: __exp2_finite, __exp_finite, __exp_finite, __log_finite
lib/libsyn123.a: _ZGVbN2v___exp_finite, __exp_finite, __exp_finite, __log10_finite, __log_finite
Please add -fno-finite-math-only to compiler flags to disable the use of these functions in optimizations. (https://github.com/conan-io/conan-center-index/blob/master/docs/error_knowledge_base.md#KB-H078-USING-GLIBC-FINITE-MATH-FUNCTIONS) 
 ---> e704ef8f3151
Step 8/8 : RUN objdump -t ~/.conan/data/mpg123/1.31.2/_/_/package/*/lib/libmpg123.a | grep _finite || true
 ---> Running in 861d11ead80f
0000000000000000         *UND*	0000000000000000 __exp_finite
0000000000000000         *UND*	0000000000000000 __log_finite
0000000000000000         *UND*	0000000000000000 __exp_finite
0000000000000000         *UND*	0000000000000000 __exp2_finite
 ---> Running in 1f07aaba5626

Step 9/10 : RUN CFLAGS=-fno-finite-math-only conan create conan-center-index/recipes/mpg123/all /1.31.2@ --build=mpg123 -pr:b=default
[...]
mpg123/1.31.2 (test package): Running test()
 ---> e6eebcae5af7
Step 10/10 : RUN objdump -t ~/.conan/data/mpg123/1.31.2/_/_/package/*/lib/libmpg123.a | grep _finite || true
 ---> Running in 41ce466a9cae

[valgur]

@uilianries, @memsharded. Have you had a chance to look at this PR? A friendly ping just in case this has slipped through the cracks.

By my count this issue affects at least 75 packages just through the transitive dependencies on mpg123, openjpeg, libx264 and libx265 alone. I would like to add the one-liner fixes to these packages and others but would like this hook to be merged first to be certain that the changes are really having an effect.

[memsharded]

Thanks for the contribution.

I am a bit concerned about the complexity of this hook check, it is a bit outside of my expertise. As we move towards 2.0 that is not using hooks in ConanCenter yet, not sure about the value, and possible risks of false positives and other issues.

Will ask for further reviews.

[uilianries]

I'm concerned about this hook, just like @memsharded said, it's complex and hard to understand, and can turn up as a nightmare in case we need to fix something in the future.

Problems related to glibc version in CCI are not new and we really can't fix all them. If we use only new Ubuntu versions, we will break users that are still running older versions, but if we only run old versions, we private some libraries due new glibc features, so we try to keep it balanced. Our suggestion is building from source in case having trouble with glibc, so you can update your package according to your scenario. But in case of real failure in CCI, we should investigate for sure.

[valgur]

I am a bit concerned about the complexity of this hook check

I'm concerned about this hook, just like @memsharded said, it's complex and hard to understand, and can turn up as a nightmare in case we need to fix something in the future.

While I empathize with the maintenance concern, I based this hook largely on "KB-H043 - MISSING SYSTEM LIBS" and feel like this hook is almost straightforward compared to that one?

• hooks/hooks/conan-center.py

  Lines 1209 to 1229 in ad69859

  	@run_test("KB-H043", output)
  	def test(out):
  	dict_deplibs_libs = _deplibs_from_shlibs(conanfile, out)
  	all_system_libs = _all_system_libs(_get_os(conanfile))
  	needed_system_libs = set(dict_deplibs_libs.keys()).intersection(all_system_libs)
  	if _get_os(conanfile) == "Macos":
  	deps_system_libs = set(conanfile.deps_cpp_info.frameworks)
  	else:
  	deps_system_libs = set(conanfile.deps_cpp_info.system_libs)
  	conanfile_system_libs = set(m.group(2) for m in re.finditer(r"""(["'])([a-zA-Z0-9._-]+)(\1)""", tools.load(conanfile_path))).intersection(all_system_libs)
  	missing_system_libs = needed_system_libs.difference(deps_system_libs.union(conanfile_system_libs))
  	attribute = 'frameworks' if _get_os(conanfile) == "Macos" else 'system_libs'
  	for missing_system_lib in missing_system_libs:
  	libs = dict_deplibs_libs[missing_system_lib]
  	for lib in libs:
  	out.warn("Library '{}' links to system library '{}' but it is not in cpp_info.{}.".format(lib, missing_system_lib, attribute))

• hooks/hooks/conan-center.py

  Lines 1536 to 1606 in ad69859

  	def _deplibs_from_shlibs(conanfile, out):
  	deplibs = dict()
  	os_ = _get_os(conanfile)
  	shlext = {
  	"Windows": "dll",
  	"Macos": "dylib"
  	}.get(os_, "so")
  	libraries = _get_files_with_extensions(conanfile.package_folder, [shlext])
  	if not libraries:
  	return deplibs
  	if os_ == "Linux" or tools.is_apple_os(os_) or _get_compiler(conanfile) not in ["Visual Studio", "msvc"]:
  	objdump = tools.get_env("OBJDUMP") or tools.which("objdump")
  	if not objdump:
  	out.warn("objdump not found")
  	return deplibs
  	for library in libraries:
  	if _get_os(conanfile) == "Windows":
  	cmd = [objdump, "--section=.idata", "-x", library]
  	else:
  	cmd = [objdump, "-p", library]
  	try:
  	objdump_output = subprocess.check_output(cmd, cwd=conanfile.package_folder).decode()
  	except subprocess.CalledProcessError:
  	out.warn("Running objdump on '{}' failed. Is the environment variable OBJDUMP correctly configured?".format(library))
  	continue
  	if _get_os(conanfile) == "Windows":
  	for dep_lib_match in re.finditer(r"DLL Name: (.*).dll", objdump_output, re.IGNORECASE):
  	dep_lib_base = dep_lib_match.group(1).lower()
  	deplibs.setdefault(dep_lib_base, []).append(library)
  	elif _get_os(conanfile) == "Macos":
  	load_commands = {}
  	number = None
  	for line in objdump_output.splitlines():
  	if line.startswith("Load command"):
  	tokens = line.split("Load command")
  	if len(tokens) == 2:
  	number = int(tokens[1])
  	load_commands[number] = dict()
  	elif number is not None:
  	line = line.strip()
  	tokens = line.split(None, 1)
  	if len(tokens) == 2:
  	load_commands[number][tokens[0]] = tokens[1]
  	r = r"/System/Library/Frameworks/(.*)\.framework/Versions/(.*)/(.*) \(offset (.*)\)"
  	r = re.compile(r)
  	for load_command in load_commands.values():
  	if load_command.get("cmd") == "LC_LOAD_DYLIB":
  	name = load_command.get("name", '')
  	match = re.match(r, name)
  	if not match:
  	continue
  	deplibs.setdefault(match.group(1), []).append(library)
  	else:
  	dep_libs_fn = list(l.replace("NEEDED", "").strip() for l in objdump_output.splitlines() if "NEEDED" in l)
  	for dep_lib_fn in dep_libs_fn:
  	dep_lib_match = re.match(r"lib(.*).{}(?:\.[0-9]+)*".format(shlext), dep_lib_fn)
  	if not dep_lib_match:
  	continue
  	deplibs.setdefault(dep_lib_match.group(1), []).append(library)
  	elif _get_compiler(conanfile) in ["Visual Studio", "msvc"] or _get_os == "Windows":
  	with tools.vcvars(conanfile):
  	for library in libraries:
  	try:
  	dumpbin_output = subprocess.check_output(["dumpbin", "-dependents", library], cwd=conanfile.package_folder).decode()
  	except subprocess.CalledProcessError:
  	out.warn("Running dumpbin on '{}' failed.".format(library))
  	continue
  	for l in re.finditer(r"([a-z0-9\-_]+)\.dll", dumpbin_output, re.IGNORECASE):
  	dep_lib_base = l.group(1).lower()
  	deplibs.setdefault(dep_lib_base, []).append(library)
  	return deplibs

Problems related to glibc version in CCI are not new and we really can't fix all them.

Are there any other examples besides this one?

I find it hard to agree if the fix for the issue at hand is to add the following to couple of lines to generate()

if not is_msvc(self):
    tc.variables["CMAKE_C_FLAGS"] = "-fno-finite-math-only"
    tc.variables["CMAKE_CXX_FLAGS"] = "-fno-finite-math-only"

as long as there's a simple way to detect this issue (which there isn't when building locally - you would have to run something similar to the above Dockerfile for each recipe to even detect the issue).

Not detecting and fixing this issue effectively means that almost none of the packages involving some kind of image or audio I/O can be built with GCC (new or old) unless everything is rebuilt locally. This includes Qt, GTK, OpenCV, GDAL, SDL and many other packages that are both popular and a pain to re-build. Why bother distributing the binaries at all if they will only work with 5+ year old OS releases?

[valgur]

And as for

possible risks of false positives

It only matches against undefined symbols in library files with two leading underscores. The language standard only allows these to be defined by the compiler and the libc itself. There's a very limited number of these around. The regex pattern could also be replaced with one that matches the exact list of relevant symbols that I linked to above.

There is one case where this hook could incorrectly flag an issue, though - pre-built external binaries. I probably should add an exception for that.
Edit: added an exception for this.

[valgur]

@memsharded @uilianries @jcar87
Please. I understand that you would prefer to not touch the hooks code at all, if possible. So I propose the following:

1. Run this test to confirm that the hook works as expected:

FROM conanio/gcc8:1.60.2

# Set-up
USER root
RUN git clone https://github.com/conan-io/conan-center-index.git --depth 1
RUN conan profile new default --detect && conan profile update settings.compiler.libcxx=libstdc++11 default
RUN mkdir -p ~/.conan/hooks/ && \
    wget https://raw.githubusercontent.com/valgur/conan-hooks/feature/forbid-glibc-finite-math-functions/hooks/conan-center.py -O ~/.conan/hooks/conan-center.py && \
    chmod +x ~/.conan/hooks/conan-center.py
RUN conan config set hooks.conan-center

# The actual test
RUN conan create conan-center-index/recipes/mpg123/all /1.31.2@ --build=mpg123 -pr:b=default

# Uncomment to verify that building with the offending optimization turned off fixes the issue
# RUN CFLAGS=-fno-finite-math-only conan create conan-center-index/recipes/mpg123/all /1.31.2@ --build=mpg123 -pr:b=default

2. Merge.
3. Revert the PR if there's a slightest hint of issues in CCI related to this hook.
4. If you ever decide to migrate hooks to Conan v2, feel free to drop this one entirely. Any packages for which this hook is relevant are most likely fixed by then anyway.

Would that be an acceptable compromise?

[valgur]

Cc @SpaceIm @ericLemanissier @toge @jwillikers
Do you agree that this is a real issue in CCI that needs a fix?

[ericLemanissier]

I agree that it's a real issue, which somehow needs to be addressed. Unfortunately, the current policy seems to be to reduce all automation changes on conan-center to the bare minimum. If this does not get merged as a hook, It'll probably has more chances to live as a custom command which would have the advantage of being compatible with conan 2. Then you'll have to find some way to make this run automatically on conan-center binaries (cf my DM on slack)

[jwillikers]

@valgur I have to agree that this is an important fix for making Conan Center Index binaries consumable by others on Linux. It should not be overlooked as it can lead to unexpected and hard to debug errors for consumers.

Unfortunately, libc handling has always been an omission with Conan. I've solved this kind of issue by using a subsetting of os for the libc type and libc version. By gatekeeping and rebuilding all binaries from CCI, this setting is always added to the Conan packages from CCI. It's not ideal since it doesn't allow using something compiled against an older glibc on a system with a newer glibc, but I guess it has the benefit of preventing somewhat rarer issues like this one.

I hope the team is able to figure out ways to improve glibc handling, like what your PR offers here. Thanks @valgur for working on this.

[AbrilRBS]

Thanks for your insight everyone :)
I've set this to be discussed with the team, thanks a lot for your patience, will let you know once I know more!