Bug 60125 - 0.9.11 sometimes fails to link due to failure to inline SortedArrayOf<>::search func
Summary: 0.9.11 sometimes fails to link due to failure to inline SortedArrayOf<>::sear...
Status: RESOLVED WORKSFORME
Alias: None
Product: HarfBuzz
Classification: Unclassified
Component: src (show other bugs)
Version: unspecified
Hardware: x86 (IA32) Linux (All)
: medium major
Assignee: Behdad Esfahbod
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-31 15:45 UTC by gmad
Modified: 2013-12-04 22:56 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
patch to fix template func inlining failure on some GCC installs (289 bytes, text/plain)
2013-01-31 15:45 UTC, gmad
Details

Description gmad 2013-01-31 15:45:33 UTC
Created attachment 74001 [details]
patch to fix template func inlining failure on some GCC installs

There's a minor problem with GCC failing to properly inline one of the templated search functions (resulting in failure to link) which is addressed by the attached patch against 0.9.11 to move the definition of the func outside the class decl.
Comment 1 Behdad Esfahbod 2013-02-05 02:05:52 UTC
Thanks.  I'll try it.  Much appreciated, since this has been an unsolved problem for a while.
Comment 2 Behdad Esfahbod 2013-02-05 03:58:43 UTC
Humm, I had to add back inline.  Without inline I consider the patch wrong.  Please see if it still helps:

diff --git a/src/hb-open-type-private.hh b/src/hb-open-type-private.hh
index 90f2836..5370c67 100644
--- a/src/hb-open-type-private.hh
+++ b/src/hb-open-type-private.hh
@@ -945,9 +945,13 @@ struct HeadlessArrayOf
 /* An array with sorted elements.  Supports binary searching. */
 template <typename Type>
 struct SortedArrayOf : ArrayOf<Type> {
-
   template <typename SearchType>
-  inline int search (const SearchType &x) const {
+  inline int search (const SearchType &x) const;
+};
+
+template <typename Type>
+template <typename SearchType>
+inline int SortedArrayOf<Type>::search (const SearchType &x) const {
     unsigned int count = this->len;
     /* Linear search is *much* faster for small counts. */
     if (likely (count < 32)) {
@@ -962,9 +966,7 @@ struct SortedArrayOf : ArrayOf<Type> {
       const Type *p = (const Type *) bsearch (&x, this->array, this->len, sizeof (this->array[0]), (hb_compare_func_t) Cmp::cmp);
       return p ? p - this->array : -1;
     }
-  }
-};
-
+}
 
 } /* namespace OT */
Comment 3 gmad 2013-02-06 05:33:54 UTC
Adding inline back in re-broke the patch for me (I again failed to link),
but using __attribute((always_inline)) instead of the weaker 'inline' seems to do the trick.

Happy compromise? ;)
Comment 4 Behdad Esfahbod 2013-02-06 19:26:28 UTC
Meh.  Yeah.  Does it work if we don't move the definition out?  And does it work if we have both inline and the attribute?
Comment 5 Behdad Esfahbod 2013-02-06 19:26:58 UTC
What are the GCC versions that you've observed this on BTW?
Comment 6 gmad 2013-02-09 15:04:11 UTC
Sorry for the delay, but here's what I found out:

- It affects gcc-4.3.4 (which I was using due to reqs of OTHER code),
  it seems to inline correctly on >= gcc-4.5.4

- Just putting the __attribute((always_inline)) on the decl within the template does not fix the problem (you get the same link issue as the plain 'inline' decl)

Probably not a necessary patch for anyone not compiling in the stone age, but maybe a nice work-around to pass along for the desperate.

Cheers!
Comment 7 Behdad Esfahbod 2013-12-04 22:56:42 UTC
No resolution.  Lets close.  Thanks.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.