1272649 - Flexible array member in union error in gfx/harfbuzz/src/hb-font-private.hh

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[u403884]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Firefox/45.0
Build ID: 20160317220330

Steps to reproduce:

Try to build firefox 45.0.1 using gcc6 (6.1.0) instead of gcc5


Actual results:

Build failure while trying to compile gfx/harfbuzz/src/hb-ot-shape-complex-hangul.cc:

c++ -o hb-ot-shape-complex-hangul.o -c -I/home/guido/new/build-firefox-45.0.1/dist/stl_wrappers -I/home/guido/new/build-firefox-45.0.1/dist/system_wrappers -include /home/guido/new/firefox-45.0.1/config/gcc_hidden.h -DPACKAGE_VERSION='"moz"' -DPACKAGE_BUGREPORT='"http://bugzilla.mozilla.org/"' -DHAVE_OT=1 -DHB_NO_MT -DHB_NO_UNICODE_FUNCS -I/home/guido/new/firefox-45.0.1/gfx/harfbuzz/src -I.  -I../../../dist/include  -I/usr/include/nspr -I/usr/include/nss3    -I/usr/include/pixman-1   -fPIC  -DMOZILLA_CLIENT -include ../../../mozilla-config.h -MD -MP -MF .deps/hb-ot-shape-complex-hangul.o.pp  -Wall -Wempty-body -Woverloaded-virtual -Wsign-compare -Wwrite-strings -Wno-invalid-offsetof -Wcast-align -O2 -march=core2 -mtune=core2 -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -std=gnu++0x -pthread -pipe  -DDEBUG -DTRACING -g -freorder-blocks -Os -fno-omit-frame-pointer   -UDEBUG   /home/guido/new/firefox-45.0.1/gfx/harfbuzz/src/hb-ot-shape-complex-hangul.cc
In file included from /home/guido/new/firefox-45.0.1/gfx/harfbuzz/src/hb-ot-layout-private.hh:34:0,
                 from /home/guido/new/firefox-45.0.1/gfx/harfbuzz/src/hb-ot-shape-private.hh:33,
                 from /home/guido/new/firefox-45.0.1/gfx/harfbuzz/src/hb-ot-shape-complex-private.hh:32,
                 from /home/guido/new/firefox-45.0.1/gfx/harfbuzz/src/hb-ot-shape-complex-hangul.cc:27:
/home/guido/new/firefox-45.0.1/gfx/harfbuzz/src/hb-font-private.hh:83:26: error: flexible array member in union
     void (*array[]) (void);
                          ^
/home/guido/new/firefox-45.0.1/config/rules.mk:956: set di istruzioni per l'obiettivo "hb-ot-shape-complex-hangul.o" non riuscito
make[3]: *** [hb-ot-shape-complex-hangul.o] Errore 1
make[3]: uscita dalla directory "/home/guido/new/build-firefox-45.0.1/gfx/harfbuzz/src"
/home/guido/new/firefox-45.0.1/config/recurse.mk:71: set di istruzioni per l'obiettivo "gfx/harfbuzz/src/target" non riuscito
make[2]: *** [gfx/harfbuzz/src/target] Errore 2



Expected results:

The build should not have failed while trying to compile gfx/harfbuzz/src/hb-ot-shape-complex-hangul.cc.

Comment 1

[u403884]

The same code, compiles fine in latest standalone harfbuzz library.

Comment 2

[u403884]

The strange thing is that exactly the same code, compiles fine within the standalone harfbuzz library.

Comment 3

[Jonathan Kew [:jfkthame]]

This should be fixed in harfbuzz 1.1.3 (see bug 1228540, which landed for mozilla-46).

If you want to patch the problem locally for a mozilla-45 build, you can just do

-     void (*array[]) (void);
+     void (*array[1]) (void);

at gfx/harfbuzz/src/hb-font-private.hh:83.

Comment 4

[u403884]

I was referring to latest standalone harfbuzz, which is 1.2.7. The same code does compile in standalone harfbuzz and not in the firefox copy of harfbuzz...

It must be some compilation flag !

I see mozilla-46 applied the following:

--- firefox-45.0.1/gfx/harfbuzz/src/hb-font-private.hh  2016-03-15 23:37:28.000000000 +0100
+++ firefox-46.0.1/gfx/harfbuzz/src/hb-font-private.hh  2016-05-03 07:31:12.000000000 +0200
@@ -80,7 +82,7 @@ struct hb_font_funcs_t {
       HB_FONT_FUNCS_IMPLEMENT_CALLBACKS
 #undef HB_FONT_FUNC_IMPLEMENT
     } f;
-    void (*array[]) (void);
+    void (*array[VAR]) (void);
   } get;
 };

Comment 5

[u403884]

And then also, as you suggested:

--- firefox-45.0.1-orig/gfx/harfbuzz/src/hb-private.hh  2016-03-15 23:37:28.000000000 +0100
+++ firefox-45.0.1/gfx/harfbuzz/src/hb-private.hh       2016-05-13 17:20:55.909655179 +0200
@@ -1010,5 +1010,7 @@ hb_options (void)
   return _hb_options.opts;
 }
 
+/* Size signifying variable-sized array */
+#define VAR 1
 
 #endif /* HB_PRIVATE_HH */

Comment 6

[info]

I has problem with compiling FF ESR 45.1.1 with GCC 6.1.1 too - I applied both patches (from comment 4 and 5) and i can compile FF again! 

Many thanks [e-mail]

Comment 7

[u403884]

I am not inclined to close this bug yet, as I would rather prefer seeing a long-term fix which doesn't change the code but rather the way it is compiled (see Comment 2, https://bugzilla.mozilla.org/show_bug.cgi?id=1272649#c2).

Comment 8

[u403884]

I am sorry, I think I told you fibs in Comment 1 and Comment 2 and thus in Comment 7...

The two header files have changed in the standalone library (it's the *.cc files that haven't changed) !

So, I suppose this bug can be closed after the two fixes in Comment 4 and Comment 5 are backported to the 45 branch or preferably a whole harfbuzz update to latest version 1.2.7 code is committed. And thanks Jonathan for pointing that out !

I have no write access to the 45 branch tree, can you Jonathan actually fix the code in the repository and close this bug ?

Comment 9

[u403884]

Attachment: firefox-45.0.1-harfbuzz-flexible-array-member-in-union.patch

Comment 10

[Jonathan Kew [:jfkthame]]

Attachment: Backport fix for erroneous 'flexible array member in union' to harfbuzz 1.1.0 on esr45, to avoid compilation failure with stricter compilers

If we're going to fix this on the esr45 branch, for the sake of people wanting to build with gcc6 (or other strict compilers), we should also remove the definition of VAR from hb-opentype-private.h, as the one in hb-private.h will cover all uses. So this is the patch I suggest for esr45, cherry-picked from upstream commit a13b023d.

Comment 11

[u403884]

Yes, Jonathan, please go for the patch in Comment 10 so that we can close this.

Comment 12

[u403884]

Comment on attachment 8752592 [details] [diff] [review]
Backport fix for erroneous 'flexible array member in union' to harfbuzz 1.1.0 on esr45, to avoid compilation failure with stricter compilers

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

Patch looks good to me if whole harfbuzz update should be avoided.

Comment 13

[Jonathan Kew [:jfkthame]]

Jeff, just wondering why the r-? You don't think we should bother to try and fix this for esr45 (I realize it'd still be up to esr drivers to decide whether to take it), or some other issue with the patch?

Comment 14

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8752592 [details] [diff] [review]
Backport fix for erroneous 'flexible array member in union' to harfbuzz 1.1.0 on esr45, to avoid compilation failure with stricter compilers

I accidentally chose '-' instead of '+'

Comment 15

[Jonathan Kew [:jfkthame]]

Comment on attachment 8752592 [details] [diff] [review]
Backport fix for erroneous 'flexible array member in union' to harfbuzz 1.1.0 on esr45, to avoid compilation failure with stricter compilers

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: Currently, the esr45 code fails to build with gcc6 because of a C++ language violation in harfbuzz code. It was fixed upstream 5 months ago (so we already have the fix in central/aurora/beta, as part of a full library update).

The patch here just cherry-picks the minimal fix from upstream. There is no change in behavior, so this avoids any potential risk of backporting the full library update just to resolve the build failure.

User impact if declined: build failure on systems with gcc6

Fix Landed on Version: on mozilla-46, we took a full harfbuzz update that includes this fix (and much more)

Risk to taking this patch (and alternatives if risky): minimal - no behavior change

String or UUID changes made by this patch: none

Comment 16

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8752592 [details] [diff] [review]
Backport fix for erroneous 'flexible array member in union' to harfbuzz 1.1.0 on esr45, to avoid compilation failure with stricter compilers

Build fix, please uplift to esr45.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr45/rev/a4acf1dd5b8f

Comment 18

[Jonathan Kew [:jfkthame]]

Closing this; it was fixed already for trunk in bug 1228540, and for esr45 by the patch landed here.