Bug 1498072 - Prevent inlining of the direct_run function. r=jfkthame
authorMike Hommey <mh+mozilla@glandium.org>
Thu, 11 Oct 2018 10:42:24 +0900
changeset 491197 02ac5668f849f1a23eb3702e468e32a0900c54f6
parent 491196 524c546c7ad5a2a73f56bb6858ffcac4f82a6088
child 491198 434f70360933449c52ece7a00a6268391f805f08
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
Bug 1498072 - Prevent inlining of the direct_run function. r=jfkthame https://github.com/silnrsi/graphite/pull/46/commits/d2c1303345c359ebc31ddf97b73711dec4ee894e Differential Revision: https://phabricator.services.mozilla.com/D8328
--- a/gfx/graphite2/src/direct_machine.cpp
+++ b/gfx/graphite2/src/direct_machine.cpp
@@ -51,16 +51,36 @@ of the License or (at your option) any l
 #define do_(name)               &&name
 using namespace graphite2;
 using namespace vm;
 namespace {
+// The GCC manual has this to say about labels as values:
+//   The &&foo expressions for the same label might have different values
+//   if the containing function is inlined or cloned. If a program relies
+//   on them being always the same, __attribute__((__noinline__,__noclone__))
+//   should be used to prevent inlining and cloning.
+// is_return in Code.cpp relies on being able to do comparisons, so it needs
+// them to be always the same.
+// The GCC manual further adds:
+//   If &&foo is used in a static variable initializer, inlining and
+//   cloning is forbidden.
+// In this file, &&foo *is* used in a static variable initializer, and it's not
+// entirely clear whether this should prevent inlining of the function or not.
+// In practice, though, clang 7 can end up inlining the function with ThinLTO,
+// which breaks at least is_return. https://bugs.llvm.org/show_bug.cgi?id=39241
+// So all in all, we need at least the __noinline__ attribute. __noclone__
+// is not supported by clang.
 const void * direct_run(const bool          get_table_mode,
                         const instr       * program,
                         const byte        * data,
                         Machine::stack_t  * stack,
                         slotref         * & __map,
                         uint8                _dir,
                         Machine::status_t & status,
                         SlotMap           * __smap=0)