On Mon, Aug 31, 2015 at 20:07:49 +0200, Jakub Jelinek wrote: > On Mon, Aug 31, 2015 at 08:56:58PM +0300, Ilya Verbin wrote: > > On Mon, Aug 31, 2015 at 16:49:53 +0200, Jakub Jelinek wrote: > > > 1) Is the library backwards ABI compatible? Can you run e.g. > > > libabigail abidiff in between the unpatched and patched version? > > > > It should be in theory, and I've successfully tested an old binary with old > > libgomp plugin and with new liboffloadmic. However, `abidiff --changed-fns > > old/liboffloadmic_host.so new/liboffloadmic_host.so` prints: > > > > Functions changes summary: 0 Removed (82 filtered out), 7 Changed (21 > > filtered out), 0 Added functions (1081 filtered out) > > Variables changes summary: 0 Removed (25 filtered out), 1 Changed, 0 Added > > variables (7 filtered out) > > Function symbols changes summary: 7 Removed, 76 Added function symbols not > > referenced by debug info > > Variable symbols changes summary: 22 Removed, 4 Added variable symbols not > > referenced by debug info > > > > 7 functions with some indirect sub-type change: > > > > /* Unused functions skipped. */ > > > > [C]'function int __offload_offload(OFFLOAD, const char*, int, int, > > VarDesc*, VarDesc2*, int, int)' has some indirect sub-type changes: > > parameter 1 of type 'typedef OFFLOAD' has sub-type changes: > > underlying type 'OffloadDescriptor*' changed: > > in pointed to type 'struct OffloadDescriptor': > > type size changed from 2240 to 2368 bits > > 9 data member insertions: > > /* ... */ > > > > [C]'function void __offload_register_image()' has some indirect sub-type > > changes: > > return type changed: > > type name changed from 'void' to 'bool' > > type size changed from 0 to 8 bits > > E.g. this is an ABI change, if e.g. anything in the plugin will call > __offload_register_image and check for return value (i.e. expect the new > versioN), but actually at runtime link against the old one, it will > misbehave.
Looks like this is the only incompatible change. Given that the library is used only by libgomp plugin, this isn't a big problem. I will add a comment into plugin prohibiting the use of the return value. In fact, if something goes wrong in __offload_register_image, it calls exit (). > > > 2) the *.map changes look wrong, when adding symbols to a symbol versioned > > > shared library, new symbols shouldn't be added to existing symbol > > > version, but to a new symbol version (so e.g. MYO_1.0.1 (or MYO_1.1) > > > and COI_1.0.1 (or COI_1.1)) > > > > I agree, but this is what I can't change - these files are copied from real > > COI/ > > MYO libraries, therefore the emulator (fake COI/MYO libs) must have the same > > versions as the real libs. > > If that change is already cast into stone, there is nothing we can do, but > can you talk to the library maintainers that they add new symbols to > different symbol version next time at least? Yes, I have told them. So, is it OK for trunk? -- Ilya
