Bug 25661 - [libabigail] Handle data member replacement by anonymous data members
Summary: [libabigail] Handle data member replacement by anonymous data members
Status: RESOLVED FIXED
Alias: None
Product: libabigail
Classification: Unclassified
Component: default (show other bugs)
Version: unspecified
: P2 enhancement
Target Milestone: ---
Assignee: Dodji Seketeli
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-03-12 12:15 UTC by David Marchand
Modified: 2020-05-18 12:28 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Marchand 2020-03-12 12:15:02 UTC
This had been caught by a patchset submitted in the dpdk project.
Here is a reproducer:

$ cat plop.c
#include <inttypes.h>

struct _export {
#ifdef BEFORE
	uint64_t marker[0];
	uint64_t a;
	uint64_t b;
#else
	union {
		uint64_t marker[0];
		struct {
			uint64_t a;
			uint64_t b;
		};
	};
#endif
	uint64_t c;
};

extern void set_a(struct _export *sym);
void set_a(struct _export *sym)
{
	sym->a = 1;
}

$ cat Makefile 
CC ?= gcc
CFLAGS ?= -Wall -Werror -g
ABIDIFF = abidiff

before_%.o: %.c
	$(CC) $(CFLAGS) -DBEFORE -o $@ -c $<
	pahole $@

after_%.o: %.c
	$(CC) $(CFLAGS) -o $@ -c $<
	pahole $@

dump: before_plop.o after_plop.o
	$(ABIDIFF) $^

[dmarchan@wsfd-netdev66 struct_anon_field]$ rm *.o; make
cc -Wall -Werror -g -DBEFORE -o before_plop.o -c plop.c
pahole before_plop.o
struct _export {
	uint64_t                   marker[0];            /*     0     0 */
	uint64_t                   a;                    /*     0     8 */
	uint64_t                   b;                    /*     8     8 */
	uint64_t                   c;                    /*    16     8 */

	/* size: 24, cachelines: 1, members: 4 */
	/* last cacheline: 24 bytes */
};
cc -Wall -Werror -g -o after_plop.o -c plop.c
pahole after_plop.o
struct _export {
	union {
		uint64_t           marker[0];            /*     0     0 */
		struct {
			uint64_t   a;                    /*     0     8 */
			uint64_t   b;                    /*     8     8 */
		};                                       /*     0    16 */
	};                                               /*     0    16 */
	uint64_t                   c;                    /*    16     8 */

	/* size: 24, cachelines: 1, members: 2 */
	/* last cacheline: 24 bytes */
};
abidiff before_plop.o after_plop.o
Functions changes summary: 0 Removed, 1 Changed, 0 Added function
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable

1 function with some indirect sub-type change:

  [C]'function void set_a(_export*)' at plop.c:21:1 has some indirect sub-type changes:
    parameter 1 of type '_export*' has sub-type changes:
      in pointed to type 'struct _export' at plop.c:3:1:
        type size hasn't changed
        2 data member deletions:
          'uint64_t _export::marker[]', at offset 0 (in bits) at plop.c:5:1

          'uint64_t _export::b', at offset 64 (in bits) at plop.c:7:1

        1 data member change:
         data member uint64_t _export::a at offset 0 (in bits) became anonymous data member 'union {uint64_t marker[]; struct {uint64_t a; uint64_t b; };}'


make: *** [Makefile:14: dump] Error 4
Comment 1 Dodji Seketeli 2020-04-29 13:30:17 UTC
Thank you for filing this issue.

I have been working on it and the work-in-progress code is in the branch at dodji/PR25661 at https://sourceware.org/git/?p=libabigail.git;a=shortlog;h=refs/heads/dodji/PR25661.

You can check it out by doing:

git clone -b dodji/PR25661 git://sourceware.org/git/libabigail.git libabigail/PR25661
Comment 2 Dodji Seketeli 2020-04-29 13:31:09 UTC
Here is what the code of the dodji/PR25661 branch does so far, on the example submitted in this problem report:

$ diff -u test5-v0.c test5-v1.c
--- test5-v0.c  2020-04-29 15:27:48.928684182 +0200
+++ test5-v1.c  2020-04-29 15:26:47.431127898 +0200
@@ -2,9 +2,15 @@

 struct S
 {
-  uint64_t marker[0];                                                                                                                                                                                                                                                          
-  uint64_t a;                                                                                                                                                                                                                                                                  
-  uint64_t b;                                                                                                                                                                                                                                                                  
+  union
+  {
+    uint64_t marker[0];                                                                                                                                                                                                                                                        
+    struct
+    {
+      uint64_t a;                                                                                                                                                                                                                                                              
+      uint64_t b;                                                                                                                                                                                                                                                              
+    };                                                                                                                                                                                                                                                                         
+  };                                                                                                                                                                                                                                                                           
   uint64_t c;                                                                                                                                                                                                                                                                  
 };                                                                                                                                                                                                                                                                             

$ build/tools/abidiff test5-v0.o test5-v1.o
Functions changes summary: 0 Removed, 0 Changed (1 filtered out), 0 Added function
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable

$ abidiff --harmless test5-v0.o test5-v1.o
Functions changes summary: 0 Removed, 1 Changed, 0 Added function
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable

1 function with some indirect sub-type change:

  [C] 'function void func(S*)' at test5-v1.c:18:1 has some indirect sub-type changes:
    parameter 1 of type 'S*' has sub-type changes:
      in pointed to type 'struct S' at test5-v1.c:3:1:
        type size hasn't changed
        data members 'S::a', 'S::marker', 'S::b' were replaced by anonymous data member:
        'union {uint64_t marker[]; struct {uint64_t a; uint64_t b;};}'                                                                                                                                                                                                          
$
Comment 3 Dodji Seketeli 2020-04-29 13:32:55 UTC
I am doing more testing and reviewing of the code base, but if you have time, you might want to test it as well.  I'll hopefully be able to submit the change soon once I am done with the testing and cleanup.
Comment 4 David Marchand 2020-04-29 16:37:30 UTC
Works for me.
Thanks!
Comment 5 Dodji Seketeli 2020-05-18 12:28:54 UTC
This should now be solved in the master branch by patches from https://sourceware.org/git/?p=libabigail.git;a=commit;h=48f26ddc00164bd0add63f933ade44e6327121a6 to https://sourceware.org/git/?p=libabigail.git;a=commit;h=5eb4d7627acb52daab6122fd8735362bbeaf19d3.

It should be available in the coming 1.8 version of Libabigail.

Thanks!