STR #3145: Shared library optimizations are breaking ABI compatibility

STR #3145

Application:	FLTK Library
Status:	1 - Closed w/Resolution
Priority:	5 - Critical, e.g. nothing working at all
Scope:	2 - Specific to an operating system
Subsystem:	DLL/DSO
Summary:	Shared library optimizations are breaking ABI compatibility
Version:	1.3-current
Created By:	AlbrechtS
Assigned To:	AlbrechtS
Fix Version:	1.3.3 (SVN: v10414)
Update Notification:	Receive EMails Don't Receive EMails

Trouble Report Files:

Trouble Report Comments:

#1	AlbrechtS 07:07 Oct 29, 2014

It turned out that the shared library size optimizations introduced recently with special compiler and linker options appear to break binary compatibility, which is a high goal for the shared libraries. This is (according to configure) only relevant for Linux (and maybe BSD) builds, IIRC. If this is true, then we should disable the optimizations with a configure option for the default builds. Specialized Linux distros can turn the option on, if this is desired. Will follow up with ABI checker reports.

#2	AlbrechtS 07:18 Oct 29, 2014

This is the report when library optimizations are enabled: $ ../abi-compliance-checker/abi-compliance-checker.pl \ > -lib FLTK \ > -old fltk-1.3.2.xml \ > -new fltk-1.3.3.xml preparation, please wait ... Using GCC 4.8.3 (x86_64-redhat-linux, target: x86_64) checking header(s) 1.3.2 ... checking header(s) 1.3.3 ... comparing ABIs ... comparing APIs ... creating compatibility report ... result: INCOMPATIBLE (Binary: 6.1%, Source: 0.03%) total "Binary" compatibility problems: 126, warnings: 13 total "Source" compatibility problems: 2, warnings: 11 see detailed report: compat_reports/FLTK/1.3.2_to_1.3.3/compat_report.html There are 126 "Binary" compatibility problems. See attached report in HTML form: compat_report_with_optimizations.html.

#3	AlbrechtS 07:36 Oct 29, 2014

I don't have the execution output handy as for the other report, but in the following attachment compat_report_without_optimizations.html there's only one relevant ABI problem, AFAICT: Removed symbol: XUtf8LookupString ( XIC ic, XKeyPressedEvent* event, char* buffer_return, int bytes_buffer, KeySym* keysym, int* status_return ) I don't know if this is relevant for ABI compatibility (fatal for running a program that maybe doesn't even use it, but was linked with the old shared library?) or only a minor problem. Can we ignore this one? Anybody? Lauri? I generated this report after manually removing all the following recently introduced shared library optimization compiler and linker flags from the generated makeinclude file (in case someone else would like to test): -fvisibility=hidden -ffunction-sections -fdata-sections -fvisibility-inlines-hidden -Wl,-Bsymbolic-functions -Wl,-gc-sections

#4	AlbrechtS 07:54 Oct 29, 2014

How to check the ABI compatibility (you may also look for the information Greg posted recently in fltk.coredev, IIRC). I was hesitant to download and install all this before, but it turned out to be very easy on a recent Linux system. I'd appreciate it someone with Mac OS X experience could do this as well on Mac OS X. Download abi-compliance-checker with git or download a distribution file and unpack it. I used git with this URL: git clone https://github.com/lvc/abi-compliance-checker.git You can "install" it (see README and/or INSTALL) or run it directly from the download directory. I did the latter. You will need two xml description files for the libraries you're checking. Here are my files: fltk-1.3.2.xml: <version> 1.3.2 </version> <headers> /path/to/fltk-1.3.2/FL/ </headers> <libs> /path/to/fltk-1.3.2/src/ </libs> fltk-1.3.3.xml: <version> 1.3.3 </version> <headers> /path/to/fltk-1.3.3/FL/ /path/to/fltk-1.3.3/src/Xutf8.h </headers> <libs> I had to remove (or rename) two platform-dependent header files, because the ABI checker would include all files from the given directories, and these don't have OS-specific guards (they are only included dependent on the platform): delete or rename: FL/mac.H and FL/win32.H It would be good to find out if there's an option to exclude files (RTFM?). Then run the command as posted above, like this: /path/to/abi-compliance-checker.pl -lib FLTK -old fltk-1.3.2.xml -new fltk-1.3.3.xml and find the HTML report that will be generated...

#5	AlbrechtS 08:09 Oct 29, 2014

I forgot to mention: of course you need two complete FLTK builds. I decided to use FLTK 1.3.2 (not 1.3.0) vs. 1.3.3, because I assume that 1.3.2 is in use on most distros, and some added symbols in 1.3.1 and 1.3.2 would only be confusing. Both builds should be configured with defaults, but including --enable-shared (obviously). Since the shared libs are built in /src, I used that in the xml config files.

#6	ianmacarthur 08:35 Oct 29, 2014

Two things: 1: Do we know which of the compile / link options is triggering the issue? That is, if we leave some of the options enabled, do we get some of the benefit, without losing ABI compatibility? One for Lauri I guess... 2: I *believe* the XUtf8LookupString() issue is a mistake of long standing; we should never have been exporting those symbols ourselves at all. We implemented "clones" of some of the X11 functions as a workaround in the early days of our UTF8 porting, and they have been in there ever since, when probably they should have been removed to use the system-provided versions from X11 (which is what we have now done, hence the symbol is flagged as missing.) However... we can *probably* assume that the user is linking X11 libs along with the fltk lib, and that the X11 lib will provide this symbol (indeed it needs to, since fltk wants to use it!) so that the user code will still resolve the symbol and work as expected.

#7	AlbrechtS 09:47 Oct 29, 2014

@Ian: 1. I don't know which options exactly cause what effect. Maybe I'll do more tests - however, my gut feeling is that the brute-force way to remove all additional compiler and linker option, i.e. make them optional, as suggested above, would be the way to go. Everybody can enable them for local builds, and the default would then be ON in the next major (ABI) release. 2. I hope so.

#8	AlbrechtS 09:51 Oct 29, 2014

Update for ABI checker: instead of deleting or renaming above-mentioned files, add this with appropriate path names to _both_ xml config files: <skip_including> /path/to/fltk-1.3.X/FL/mac.H /path/to/fltk-1.3.X/FL/win32.H </skip_including> I added it after the <headers> section, but that's likely not important. This is obviously for checking on a Linux system; adjust the filenames for other platforms as needed.

#9	cand 12:30 Oct 29, 2014

> Removed symbol: > XUtf8LookupString > > I don't know if this is relevant for ABI compatibility (fatal for running a program that maybe doesn't even use it, but was linked with the old shared library?) or only a minor problem. Can we ignore this one? Anybody? Lauri? As Ian said, exporting it was a mistake. Un-exporting it does not affect any application that did not call the function - for example fluid-shared from 1.3.2 will function fine against the new library. > missing new symbols with optimizations This is due to missing export notation in the new files (fl_copy_surface.h etc). This also affects Windows, and so would be a release blocker - those notations need to be added, or the shared Windows DLL won't export the new classes. > the problems Checking those now.

#10	cand 12:44 Oct 29, 2014

OK, majority of the "missing symbols" are inlines - those won't cause trouble, their code was inlined to the apps before and will in the future. Some are wrongly placed functions like _fl_filename_isdir_quick. Even the comment for that says it's for FLTK internal use only, yet it's in a public header. The remainder are missing export notations (or missing #include of the header), which would also break Windows.

#11	AlbrechtS 18:01 Oct 29, 2014

Lauri, I saw that you committed a few changes to export classes and functions. AFAICT this didn't change anything WRT our ABI check errors. Although I believe that we need the missing FL_EXPORT's, I also wanted to have a "plan B", so I added a configure option (as a patch only, not yet committed), to see which effect this would have. And now I can see that only the removed "visibility" options are enough to restore the "removed symbols". See attached file visibility.patch.

#12	AlbrechtS 18:04 Oct 29, 2014

This patch adds the configure option --enable-shared-opt with default OFF. This disables these compiler switches: -fvisibility=hidden -fvisibility-inlines-hidden This is sufficient to restore the missing symbols. So we have a plan B if nothing else helps.

#13	AlbrechtS 18:11 Oct 29, 2014

Here is the new check output: Using GCC 4.8.3 (x86_64-redhat-linux, target: x86_64) checking header(s) 1.3.2 ... checking header(s) 1.3.3 ... comparing ABIs ... comparing APIs ... creating compatibility report ... result: INCOMPATIBLE (Binary: 0.6%, Source: 0.03%) total "Binary" compatibility problems: 2, warnings: 13 total "Source" compatibility problems: 2, warnings: 11 see detailed report: compat_reports/FLTK/1.3.2_to_1.3.3/compat_report.html See attached file compat_report_disable-shared-opt.html

#14	cand 02:26 Oct 30, 2014

Anything removed by -fvisibility-inlines-hidden should be removed and would cause no issue. So for a less-noise comparison, add that to CXXFLAGS for 1.3.2. I'm attaching a report made with that. It only has 25 removed symbols: - _fl_filename_isdir_quick and fl_text_drag_me are internal functions - Fl_PostScript_Printer is an internal class like Fl_System_Printer - all remainders are from xutf8 and removed on purpose So with the two more commits I made today, the shared library side looks good to go.

#15	AlbrechtS 06:46 Nov 03, 2014

Fixed in Subversion repository. Sorry for the late reply, and thanks for the updates. Report looks good. Closing now.