Attachment 347156 Details for Bug 188598 – Patch

[patch] Patch

bug-188598-20180815125607.patch (text/plain), 16.67 KB, created by Guillaume Emont on 2018-08-15 03:56:08 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Guillaume Emont

Created: 2018-08-15 03:56:08 PDT

Size: 16.67 KB

patch

obsolete

>Subversion Revision: 234801
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index dfea2857d6c330b232bd5d6ced981e34e94d6322..1ea9b1e961d9baebfad860bb6fa36f183cd569b9 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,44 @@
>+2018-08-15  Guillaume Emont  <guijemont@igalia.com>
>+
>+        [JSC] Remove gcc warnings on mips and armv7
>+        https://bugs.webkit.org/show_bug.cgi?id=188598
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Remove many gcc/clang warnings that are false positives, mostly
>+        alignment issues.
>+
>+        * assembler/MacroAssemblerPrinter.cpp:
>+        (JSC::Printer::printMemory): a Memory keeps size information, so we
>+        know the original value was of the same size and can ignore the
>+        -Wcast-align warning.
>+        * assembler/testmasm.cpp:
>+        (JSC::floatOperands): marked as potentially unused as it is not used
>+        on all platforms.
>+        (JSC::testProbeModifiesStackValues): modifiedFlags is not used on
>+        mips, so don't declare it.
>+        * interpreter/Interpreter.cpp:
>+        (JSC::Interpreter::executeProgram):
>+        (JSC::Interpreter::executeCall):
>+        (JSC::Interpreter::executeConstruct):
>+        (JSC::Interpreter::prepareForRepeatCall):
>+        (JSC::Interpreter::execute):
>+        (JSC::Interpreter::executeModuleProgram):
>+        The prepareForExecution() method either returns nullptr or an
>+        Exception * cast to a JSObject *. It is therefore safe to cast it back
>+        to Exception * even though it has stricter alignment requirements.
>+        * jit/JITOperations.cpp:
>+        Same as for Interpreter.cpp.
>+        * runtime/JSBigInt.cpp:
>+        (JSC::JSBigInt::dataStorage):
>+        Pointer arithmetics done correctly, but gcc doesn't understand this,
>+        so we ignore -Wcast-align
>+        * tools/JSDollarVM.cpp:
>+        (JSC::codeBlockFromArg):
>+        After the cast, a check is done to make sure that the pointer is to a
>+        known CodeBlock before using it, so if the result is badly aligned it
>+        won't be used anyways.
>+
> 2018-08-12  Karo Gyoker  <karogyoker2+webkit@gmail.com>
> 
>         Disable JIT on IA-32 without SSE2
>diff --git a/Source/WTF/ChangeLog b/Source/WTF/ChangeLog
>index 508d804da95be3ff185b9bd12ae917e71469f580..a25840f235e6732d11f02ef21e5aef36bdbfbce9 100644
>--- a/Source/WTF/ChangeLog
>+++ b/Source/WTF/ChangeLog
>@@ -1,3 +1,16 @@
>+2018-08-15  Guillaume Emont  <guijemont@igalia.com>
>+
>+        [JSC] Remove gcc warnings on mips and armv7
>+        https://bugs.webkit.org/show_bug.cgi?id=188598
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Added IGNORE_WARNING() and IGNORE_WARNING_END() macros to be able to
>+        simply ignore a certain gcc/clang warning for a well defined code
>+        subset.
>+
>+        * wtf/Compiler.h:
>+
> 2018-08-10  Ryosuke Niwa  <rniwa@webkit.org>
> 
>         [macOS] Multiple third party apps crash due to the thread safety check in TimerBase::setNextFireTime
>diff --git a/Source/bmalloc/ChangeLog b/Source/bmalloc/ChangeLog
>index 81ccbee77df5229a0bf3f4374b4a2afa7a1c3d3c..1fe12c8551d0d090ada0e49da4ca0051e589235b 100644
>--- a/Source/bmalloc/ChangeLog
>+++ b/Source/bmalloc/ChangeLog
>@@ -1,3 +1,19 @@
>+2018-08-15  Guillaume Emont  <guijemont@igalia.com>
>+
>+        [JSC] Remove gcc warnings on mips and armv7
>+        https://bugs.webkit.org/show_bug.cgi?id=188598
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Use IGNORE_WARNING/IGNORE_WARNING_END to allow some casts after
>+        pointer calculations without triggering alignment warnings on mips and
>+        armv7.
>+
>+        * bmalloc/IsoDirectoryPageInlines.h:
>+        (bmalloc::IsoDirectoryPage<Config>::pageFor):
>+        * bmalloc/IsoPageInlines.h:
>+        (bmalloc::IsoPage<Config>::startAllocating):
>+
> 2018-07-27  Mark Lam  <mark.lam@apple.com>
> 
>         Initialize bmalloc::DebugHeap::m_pageSize for non-Darwin builds.
>diff --git a/Source/JavaScriptCore/assembler/MacroAssemblerPrinter.cpp b/Source/JavaScriptCore/assembler/MacroAssemblerPrinter.cpp
>index 3ed64b90ba5fbc93b2c992b40ec21f0710d15dd7..4f446239292033ebb938a8de5e7e9c26cad411b8 100644
>--- a/Source/JavaScriptCore/assembler/MacroAssemblerPrinter.cpp
>+++ b/Source/JavaScriptCore/assembler/MacroAssemblerPrinter.cpp
>@@ -137,6 +137,10 @@ void printMemory(PrintStream& out, Context& context)
>             out.printf("%p:<0x%02x %d>", p, *p, *p);
>             return;
>         }
>+        // assuming memory is not malformed, it originately pointed to a value
>+        // of the required size, which should be properly aligned on platforms
>+        // that require it.
>+        IGNORE_WARNING("-Wcast-align");
>         if (memory.numBytes == sizeof(int16_t)) {
>             auto p = reinterpret_cast<int16_t*>(ptr);
>             out.printf("%p:<0x%04x %d>", p, *p, *p);
>@@ -152,6 +156,7 @@ void printMemory(PrintStream& out, Context& context)
>             out.printf("%p:<0x%016" PRIx64 " %" PRId64 ">", p, *p, *p);
>             return;
>         }
>+        IGNORE_WARNING_END;
>         // Else, unknown word size. Fall thru and dump in the generic way.
>     }
> 
>diff --git a/Source/JavaScriptCore/assembler/testmasm.cpp b/Source/JavaScriptCore/assembler/testmasm.cpp
>index 7fef2dbe0e2b0c9ba5ccb801719aa8b48b6f9ff1..524a19a641cd52aa8bf6853299edefbe3eb69f24 100644
>--- a/Source/JavaScriptCore/assembler/testmasm.cpp
>+++ b/Source/JavaScriptCore/assembler/testmasm.cpp
>@@ -246,7 +246,7 @@ static Vector<double> doubleOperands()
> }
> 
> 
>-static Vector<float> floatOperands()
>+static Vector<float> UNUSED_FUNCTION floatOperands()
> {
>     return Vector<float> {
>         0,
>@@ -747,7 +747,9 @@ void testProbeModifiesStackValues()
>     CPUState originalState;
>     void* originalSP { nullptr };
>     void* newSP { nullptr };
>+#if !(CPU(MIPS))
>     uintptr_t modifiedFlags { 0 };
>+#endif
>     size_t numberOfExtraEntriesToWrite { 10 }; // ARM64 requires that this be 2 word aligned.
> 
> #if CPU(X86) || CPU(X86_64)
>diff --git a/Source/JavaScriptCore/interpreter/Interpreter.cpp b/Source/JavaScriptCore/interpreter/Interpreter.cpp
>index e8e0b50a391674a61e60aa8e31d4f39b49135f11..60e997d57c959bca2e976c2ee5e9cd032ca8e0fa 100644
>--- a/Source/JavaScriptCore/interpreter/Interpreter.cpp
>+++ b/Source/JavaScriptCore/interpreter/Interpreter.cpp
>@@ -939,7 +939,9 @@ failedJSONP:
>     {
>         CodeBlock* tempCodeBlock;
>         JSObject* error = program->prepareForExecution<ProgramExecutable>(vm, nullptr, scope, CodeForCall, tempCodeBlock);
>+        IGNORE_WARNING("-Wcast-align");
>         EXCEPTION_ASSERT(throwScope.exception() == reinterpret_cast<Exception*>(error));
>+        IGNORE_WARNING_END;
>         if (UNLIKELY(error))
>             return checkedReturn(error);
>         codeBlock = jsCast<ProgramCodeBlock*>(tempCodeBlock);
>@@ -997,7 +999,9 @@ JSValue Interpreter::executeCall(CallFrame* callFrame, JSObject* function, CallT
>     if (isJSCall) {
>         // Compile the callee:
>         JSObject* compileError = callData.js.functionExecutable->prepareForExecution<FunctionExecutable>(vm, jsCast<JSFunction*>(function), scope, CodeForCall, newCodeBlock);
>+        IGNORE_WARNING("-Wcast-align");
>         EXCEPTION_ASSERT(throwScope.exception() == reinterpret_cast<Exception*>(compileError));
>+        IGNORE_WARNING_END;
>         if (UNLIKELY(!!compileError))
>             return checkedReturn(compileError);
> 
>@@ -1064,7 +1068,9 @@ JSObject* Interpreter::executeConstruct(CallFrame* callFrame, JSObject* construc
>     if (isJSConstruct) {
>         // Compile the callee:
>         JSObject* compileError = constructData.js.functionExecutable->prepareForExecution<FunctionExecutable>(vm, jsCast<JSFunction*>(constructor), scope, CodeForConstruct, newCodeBlock);
>+        IGNORE_WARNING("-Wcast-align");
>         EXCEPTION_ASSERT(throwScope.exception() == reinterpret_cast<Exception*>(compileError));
>+        IGNORE_WARNING_END;
>         if (UNLIKELY(!!compileError))
>             return checkedReturn(compileError);
> 
>@@ -1112,7 +1118,9 @@ CallFrameClosure Interpreter::prepareForRepeatCall(FunctionExecutable* functionE
>     // Compile the callee:
>     CodeBlock* newCodeBlock;
>     JSObject* error = functionExecutable->prepareForExecution<FunctionExecutable>(vm, function, scope, CodeForCall, newCodeBlock);
>+    IGNORE_WARNING("-Wcast-align");
>     EXCEPTION_ASSERT(throwScope.exception() == reinterpret_cast<Exception*>(error));
>+    IGNORE_WARNING_END;
>     if (UNLIKELY(error))
>         return CallFrameClosure();
>     newCodeBlock->m_shouldAlwaysBeInlined = false;
>@@ -1170,7 +1178,9 @@ JSValue Interpreter::execute(EvalExecutable* eval, CallFrame* callFrame, JSValue
>     {
>         CodeBlock* tempCodeBlock;
>         JSObject* compileError = eval->prepareForExecution<EvalExecutable>(vm, nullptr, scope, CodeForCall, tempCodeBlock);
>+        IGNORE_WARNING("-Wcast-align");
>         EXCEPTION_ASSERT(throwScope.exception() == reinterpret_cast<Exception*>(compileError));
>+        IGNORE_WARNING_END;
>         if (UNLIKELY(!!compileError))
>             return checkedReturn(compileError);
>         codeBlock = jsCast<EvalCodeBlock*>(tempCodeBlock);
>@@ -1293,7 +1303,9 @@ JSValue Interpreter::executeModuleProgram(ModuleProgramExecutable* executable, C
>     {
>         CodeBlock* tempCodeBlock;
>         JSObject* compileError = executable->prepareForExecution<ModuleProgramExecutable>(vm, nullptr, scope, CodeForCall, tempCodeBlock);
>+        IGNORE_WARNING("-Wcast-align");
>         EXCEPTION_ASSERT(throwScope.exception() == reinterpret_cast<Exception*>(compileError));
>+        IGNORE_WARNING_END;
>         if (UNLIKELY(!!compileError))
>             return checkedReturn(compileError);
>         codeBlock = jsCast<ModuleProgramCodeBlock*>(tempCodeBlock);
>diff --git a/Source/JavaScriptCore/jit/JITOperations.cpp b/Source/JavaScriptCore/jit/JITOperations.cpp
>index 226c20e2e4ecff5bba810620684b011564713552..bc2ad8b666415d4bade2ce1d9286916640df8d9d 100644
>--- a/Source/JavaScriptCore/jit/JITOperations.cpp
>+++ b/Source/JavaScriptCore/jit/JITOperations.cpp
>@@ -1082,7 +1082,9 @@ SlowPathReturnType JIT_OPERATION operationLinkCall(ExecState* execCallee, CallLi
> 
>         CodeBlock** codeBlockSlot = execCallee->addressOfCodeBlock();
>         JSObject* error = functionExecutable->prepareForExecution<FunctionExecutable>(*vm, callee, scope, kind, *codeBlockSlot);
>+        IGNORE_WARNING("-Wcast-align");
>         EXCEPTION_ASSERT(throwScope.exception() == reinterpret_cast<Exception*>(error));
>+        IGNORE_WARNING_END;
>         if (error)
>             return handleThrowException();
>         codeBlock = *codeBlockSlot;
>@@ -1138,7 +1140,9 @@ void JIT_OPERATION operationLinkDirectCall(ExecState* exec, CallLinkInfo* callLi
>         RELEASE_ASSERT(isCall(kind) || functionExecutable->constructAbility() != ConstructAbility::CannotConstruct);
>         
>         JSObject* error = functionExecutable->prepareForExecution<FunctionExecutable>(*vm, callee, scope, kind, codeBlock);
>+        IGNORE_WARNING("-Wcast-align");
>         EXCEPTION_ASSERT_UNUSED(throwScope, throwScope.exception() == reinterpret_cast<Exception*>(error));
>+        IGNORE_WARNING_END;
>         if (error)
>             return;
>         unsigned argumentStackSlots = callLinkInfo->maxNumArguments();
>@@ -1188,7 +1192,9 @@ inline SlowPathReturnType virtualForWithFunction(
> 
>         CodeBlock** codeBlockSlot = execCallee->addressOfCodeBlock();
>         JSObject* error = functionExecutable->prepareForExecution<FunctionExecutable>(*vm, function, scope, kind, *codeBlockSlot);
>+        IGNORE_WARNING("-Wcast-align");
>         EXCEPTION_ASSERT(throwScope.exception() == reinterpret_cast<Exception*>(error));
>+        IGNORE_WARNING_END;
>         if (error) {
>             return encodeResult(
>                 vm->getCTIStub(throwExceptionFromCallSlowPathGenerator).retaggedCode<JSEntryPtrTag>().executableAddress(),
>diff --git a/Source/JavaScriptCore/runtime/JSBigInt.cpp b/Source/JavaScriptCore/runtime/JSBigInt.cpp
>index ba355bd743a3e8764809a8f27d39eea4b69e6a3f..bc3cf44c6ea3c8aeb94d97f421f44f9beeeaac20 100644
>--- a/Source/JavaScriptCore/runtime/JSBigInt.cpp
>+++ b/Source/JavaScriptCore/runtime/JSBigInt.cpp
>@@ -1312,7 +1312,13 @@ JSBigInt* JSBigInt::parseInt(ExecState* exec, VM& vm, CharType* data, unsigned l
> 
> inline JSBigInt::Digit* JSBigInt::dataStorage()
> {
>+// offsetOfData() makes sure that its return value is aligned to the size of
>+// Digit, so even though we cast to char* for pointer arithmetics, the cast to
>+// Digit* is properly aligned, though the compiler doesn't know about it,
>+// therefore we disable this warning.
>+    IGNORE_WARNING("-Wcast-align");
>     return reinterpret_cast<Digit*>(reinterpret_cast<char*>(this) + offsetOfData());
>+    IGNORE_WARNING_END;
> }
> 
> inline JSBigInt::Digit JSBigInt::digit(unsigned n)
>diff --git a/Source/JavaScriptCore/tools/JSDollarVM.cpp b/Source/JavaScriptCore/tools/JSDollarVM.cpp
>index dde310911c0dc43c1b67c35292d108677fbd2295..91b008abab31745ca8424ad5a29c2f45eb7443e4 100644
>--- a/Source/JavaScriptCore/tools/JSDollarVM.cpp
>+++ b/Source/JavaScriptCore/tools/JSDollarVM.cpp
>@@ -1382,8 +1382,16 @@ static CodeBlock* codeBlockFromArg(ExecState* exec)
>                 candidateCodeBlock = nullptr;
>             else
>                 candidateCodeBlock = func->jsExecutable()->eitherCodeBlock();
>-        } else
>+        } else {
>+            // Here it's OK to ignore the alignment requirements since
>+            // candidateCodeBlock will only be accessed if
>+            // VMInspector::isValidCodeBlock() returns true, which only happens
>+            // if the address is that of a known existing, properly aligned
>+            // codeBlock.
>+            IGNORE_WARNING("-Wcast-align");
>             candidateCodeBlock = reinterpret_cast<CodeBlock*>(value.asCell());
>+            IGNORE_WARNING_END;
>+        }
>     }
> 
>     if (candidateCodeBlock && VMInspector::isValidCodeBlock(exec, candidateCodeBlock))
>diff --git a/Source/WTF/wtf/Compiler.h b/Source/WTF/wtf/Compiler.h
>index 798c3090c9b1c18580a3a6be6fab65bdace6ffaa..ba23084c360abad3eca16df14966e7fb6b5073f2 100644
>--- a/Source/WTF/wtf/Compiler.h
>+++ b/Source/WTF/wtf/Compiler.h
>@@ -385,4 +385,23 @@
> #define __has_include(path) 0
> #endif
> 
>+/* IGNORE_WARNING */
>+
>+#if !defined(IGNORE_WARNING) && COMPILER(GCC_OR_CLANG)
>+#define _WEBKIT_IGNORE_WARNING_STRINGIFY(a) #a
>+#define IGNORE_WARNING(warning) do {\
>+    _Pragma("GCC diagnostic push") \
>+    _Pragma(_WEBKIT_IGNORE_WARNING_STRINGIFY(GCC diagnostic ignored warning)) \
>+    } while (false)
>+
>+#define IGNORE_WARNING_END do {\
>+    _Pragma("GCC diagnostic pop") \
>+    } while (false)
>+#endif
>+
>+#if !defined(IGNORE_WARNING)
>+#define IGNORE_WARNING do { } while (false)
>+#define IGNORE_WARNING_END do { } while (false)
>+#endif
>+
> #endif /* WTF_Compiler_h */
>diff --git a/Source/bmalloc/bmalloc/IsoDirectoryPageInlines.h b/Source/bmalloc/bmalloc/IsoDirectoryPageInlines.h
>index a149e108d137c941e900b573e98fd35e1b014b98..105689be0afb86004708b57e3871037433d7a7c7 100644
>--- a/Source/bmalloc/bmalloc/IsoDirectoryPageInlines.h
>+++ b/Source/bmalloc/bmalloc/IsoDirectoryPageInlines.h
>@@ -39,8 +39,13 @@ IsoDirectoryPage<Config>::IsoDirectoryPage(IsoHeapImpl<Config>& heap, unsigned i
> template<typename Config>
> IsoDirectoryPage<Config>* IsoDirectoryPage<Config>::pageFor(IsoDirectory<Config, numPages>* payload)
> {
>+    // the char * cast is only used to do a pointer calculation, and said
>+    // calculation results in a pointer to an existing, correctly aligned
>+    // IsoDirectoryPage.
>+    IGNORE_WARNING("-Wcast-align");
>     return reinterpret_cast<IsoDirectoryPage<Config>*>(
>         reinterpret_cast<char*>(payload) - BOFFSETOF(IsoDirectoryPage, payload));
>+    IGNORE_WARNING_END;
> }
> 
> } // namespace bmalloc
>diff --git a/Source/bmalloc/bmalloc/IsoPageInlines.h b/Source/bmalloc/bmalloc/IsoPageInlines.h
>index 0c47864c7b49a0eec8a2610cef67fa328a4ef4c4..c3e1edce5ddab9ae36a6c4f847d0d03a01829205 100644
>--- a/Source/bmalloc/bmalloc/IsoPageInlines.h
>+++ b/Source/bmalloc/bmalloc/IsoPageInlines.h
>@@ -188,7 +188,20 @@ FreeList IsoPage<Config>::startAllocating()
>         char* cellByte = reinterpret_cast<char*>(this) + index * Config::objectSize;
>         if (verbose)
>             fprintf(stderr, "%p: putting %p on free list.\n", this, cellByte);
>+
>+#if CPU(ARM) || CPU(MIPS)
>+        // the cast below is safe on these platforms as long as
>+        // both Config::objectSize and the alignment of this are multiples of
>+        // the required alignment of FreeCell
>+        static_assert(!(Config::objectSize % alignof(FreeCell)), "Config::objectSize should respect alignment of FreeCell");
>+        static_assert(!(alignof(IsoPage<Config>) % alignof(FreeCell)), "Alignment of IsoPage<Config> should match that of FreeCell");
>+
>+        IGNORE_WARNING("-Wcast-align");
>+#endif
>         FreeCell* cell = reinterpret_cast<FreeCell*>(cellByte);
>+#if CPU(ARM) || CPU(MIPS)
>+        IGNORE_WARNING_END;
>+#endif
>         cell->setNext(head, secret);
>         head = cell;
>         bytes += Config::objectSize;

Actions: View | Formatted Diff | Diff

Attachments on bug 188598: 347156 | 347180 | 348927 | 349562 | 349756 | 350104 | 351282 | 351289 | 351974 | 352124