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

RESOLVED FIXED

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: u403884, Assigned: jfkthame)

Tracking

45 Branch
All
Unspecified
Points:
---

Firefox Tracking Flags

(firefox-esr45 fixed)

Details

(Whiteboard: gfx-noted)

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
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.
(Reporter)

Updated

2 years ago
Hardware: Unspecified → All
(Reporter)

Comment 1

2 years ago
The same code, compiles fine in latest standalone harfbuzz library.
(Reporter)

Comment 2

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

Updated

2 years ago
Component: Untriaged → Graphics: Text
Product: Firefox → Core
(Assignee)

Comment 3

2 years ago
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.
(Reporter)

Comment 4

2 years ago
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;
 };
(Reporter)

Comment 5

2 years ago
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 */
Whiteboard: gfx-noted

Comment 6

2 years ago
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 g.trentalancia@libero.it
(Reporter)

Comment 7

2 years ago
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).
(Reporter)

Comment 8

2 years ago
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 ?
(Reporter)

Comment 9

2 years ago
Created attachment 8752504 [details] [diff] [review]
firefox-45.0.1-harfbuzz-flexible-array-member-in-union.patch
(Assignee)

Comment 10

2 years ago
Created 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

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.
Attachment #8752592 - Flags: review?(jmuizelaar)
(Assignee)

Updated

2 years ago
Assignee: nobody → jfkthame
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
(Reporter)

Comment 11

2 years ago
Yes, Jonathan, please go for the patch in Comment 10 so that we can close this.
Attachment #8752592 - Flags: review?(jmuizelaar) → review-
(Reporter)

Comment 12

2 years ago
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.
(Assignee)

Comment 13

2 years ago
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?
Flags: needinfo?(jmuizelaar)
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 '+'
Flags: needinfo?(jmuizelaar)
Attachment #8752592 - Flags: review- → review+
(Assignee)

Comment 15

2 years ago
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
Attachment #8752592 - Flags: approval-mozilla-esr45?
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.
Attachment #8752592 - Flags: approval-mozilla-esr45? → approval-mozilla-esr45+

Comment 17

2 years ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-esr45/rev/a4acf1dd5b8f
status-firefox-esr45: --- → fixed
(Assignee)

Comment 18

2 years ago
Closing this; it was fixed already for trunk in bug 1228540, and for esr45 by the patch landed here.
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.