966270 - # of constructors no longer being measured correctly

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Nathan Froyd [:froydnj]]

According to:

http://mzl.la/1ecmWFC

bug 938510 completely eliminated static constructors on Linux builds.  This is totally bogus.

The problem is that:

http://hg.mozilla.org/build/tools/file/tip/buildfarm/utils/count_ctors.py

checks for ctors first and then init_array.  But the libxul.so produced now has both and .ctors has nothing of interest in it:

@nightcrawler:~/Desktop/firefox$ readelf -W -S libxul.so
There are 33 section headers, starting at offset 0x34eabc8:

Section Headers:
  [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
  [ 0]                   NULL            00000000 000000 000000 00      0   0  0
  [ 1] .note.gnu.build-id NOTE            00000114 000114 000024 00   A  0   0  4
  [ 2] .hash             HASH            00000138 000138 011e28 04   A  3   0  4
  [ 3] .dynsym           DYNSYM          00011f60 011f60 027770 10   A  4   1  4
  [ 4] .dynstr           STRTAB          000396d0 0396d0 068bc0 00   A  0   0  1
  [ 5] .gnu.version      VERSYM          000a2290 0a2290 004eee 02   A  3   0  2
  [ 6] .gnu.version_d    VERDEF          000a7180 0a7180 000038 00   A  4   2  4
  [ 7] .gnu.version_r    VERNEED         000a71b8 0a71b8 000520 00   A  4  13  4
  [ 8] .rel.dyn          REL             000a76d8 0a76d8 27a860 08   A  3   0  4
  [ 9] .rel.plt          REL             00321f38 321f38 00b900 08   A  3  11  4
  [10] .init             PROGBITS        0032d838 32d838 000030 00  AX  0   0  4
  [11] .plt              PROGBITS        0032d870 32d870 017210 04  AX  0   0 16
  [12] .text             PROGBITS        00344a80 344a80 1aae218 00  AX  0   0 16
  [13] .fini             PROGBITS        01df2c98 1df2c98 00001c 00  AX  0   0  4
  [14] .rodata           PROGBITS        01df2d00 1df2d00 ce856c 00   A  0   0 256
  [15] .eh_frame_hdr     PROGBITS        02adb26c 2adb26c 15f274 00   A  0   0  4
  [16] .eh_frame         PROGBITS        02c3b4e0 2c3a4e0 67c1f8 00  WA  0   0  4
  [17] .tbss             NOBITS          032b76d8 32b66d8 000004 00 WAT  0   0  4
  [18] .init_array       INIT_ARRAY      032b76d8 32b66d8 0001b8 00  WA  0   0  4
  [19] .ctors            PROGBITS        032b7890 32b6890 000008 00  WA  0   0  4
  [20] .dtors            PROGBITS        032b7898 32b6898 000008 00  WA  0   0  4
  [21] .jcr              PROGBITS        032b78a0 32b68a0 000004 00  WA  0   0  4
  [22] .data.rel.ro      PROGBITS        032b78c0 32b68c0 1ac35c 00  WA  0   0 32
  [23] .dynamic          DYNAMIC         03463c1c 3462c1c 000218 08  WA  4   0  4
  [24] .got              PROGBITS        03463e34 3462e34 000830 04  WA  0   0  4
  [25] .got.plt          PROGBITS        03464664 3463664 005c8c 04  WA  0   0  4
  [26] .data             PROGBITS        0346a300 3469300 081758 00  WA  0   0 32
  [27] .bss              NOBITS          034eba60 34eaa58 16cf68 00  WA  0   0 32
  [28] .comment          PROGBITS        00000000 34eaa58 00003d 01  MS  0   0  1
  [29] .gnu_debuglink    PROGBITS        00000000 34eaa95 000014 00      0   0  1
  [30] .shstrtab         STRTAB          00000000 34eaaa9 00011f 00      0   0  1
  [31] .symtab           SYMTAB          00000000 34eb0f0 3ce8d0 10     32 239386  4
  [32] .strtab           STRTAB          00000000 38b99c0 de0e64 00      0   0  1
Key to Flags:
  W (write), A (alloc), X (execute), M (merge), S (strings)
  I (info), L (link order), G (group), T (TLS), E (exclude), x (unknown)
  O (extra OS processing required) o (OS specific), p (processor specific)
@nightcrawler:~/Desktop/firefox$ readelf -x 19 libxul.so

Hex dump of section '.ctors':
  0x032b7890 ffffffff 00000000                   ........

I think the order of the checks should be switched anyway, since .init_array is the new hotness.  Or we could check to see whether |have(.ctors) && have(.init_array) && sizeof(.ctors) == 2| or somesuch.

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: fix count-ctors.py to cope with .ctors and .init_array in the same binary

This patch totally rewrites count_ctors.py to ideally make it a little clearer what's going on, fix bugs, and generally make it more robust.

Testing locally with local binaries and libxul.so from before and after the GCC switchover indicates the changes work.

Trevor is probably the best person to review this that I can think of; r?bhearsum because I don't know if I need r+ from somebody inside releng.  Feel free to cancel or redirect as appropriate, both of you.

Comment 2

[Nathan Froyd [:froydnj]]

Attachment: fix count-ctors.py to cope with .ctors and .init_array in the same binary

Whoops, I had made some changes that I forgot to test; let's review this one instead.  Please see comment 1 for extra information.

Comment 3

[Trevor Saunders (:tbsaunde)]

Comment on attachment 8368623 [details] [diff] [review]
fix count-ctors.py to cope with .ctors and .init_array in the same binary

I'm pretty sure glandium's better, but this looks very reasonable to me, and unbreaking the test should probably happen soon.

Comment 4

[Mike Hommey [:glandium]]

Comment on attachment 8368623 [details] [diff] [review]
fix count-ctors.py to cope with .ctors and .init_array in the same binary

Review of attachment 8368623 [details] [diff] [review]:
-----------------------------------------------------------------

::: buildfarm/utils/count_ctors.py
@@ +6,5 @@
>  def count_ctors(filename):
>      proc = subprocess.Popen(
>          ['readelf', '-W', '-S', filename], stdout=subprocess.PIPE)
>  
> +    # Some versions of GCC produce both .init_array and .ctors.  So we have

Some versions of ld.

Comment 5

[Nathan Froyd [:froydnj]]

Talked to bhearsum yesterday on IRC and he said he was fine with Trevor's review for this particular file.  I applied Mike's feedback and pushed:

https://hg.mozilla.org/build/tools/rev/35ba82065362