Bug 21783 – Need more granular control over allocation of executable memory

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Oliver Hunt]

Currently fastMalloc automagically makes all pages executable to be win2k3 compatible and this is obviously less than ideal.

We only *need* executable memory for the final piece of memory that the JitCodeBuffer produces

[Mark Rowe (bdash)]

<rdar://problem/6309878>

[Oliver Hunt]

Created attachment 25808 [details]
Patch o' doom

First pass at this forsaken thing

[Cameron Zwarich (cpst)]

Comment on attachment 25808 [details]
Patch o' doom

> ExecutablePool::Allocation alloc = {(char*)mmap(NULL, n, PROT_READ | PROT_WRITE | PROT_EXEC, MAP_PRIVATE | MAP_ANON, -1, 0), n};

You don't need to cast the result of a void* memory allocation, but it seems to be our style to do so. This should be a C++ style cast.

>class ExecutablePool : public RefCounted<ExecutablePool> {
>    struct Allocation {
>        char* pages;
>        size_t size;
>    };
>    typedef Vector<Allocation, 2> AllocationList;

Do we rely on implicit private: at the beginning of class definitions?

>          if (n < (size_t)(m_end - m_freePtr)) {

Another C style cast that should be changed.

>         if ((allocSize - n) > (size_t)(m_end - m_freePtr)) {

Another.

>      if ((((size_t)-1) - ExecutableAllocator::pageSize) <= request)

You should be using std::numeric_limits<size_t>::max() instead of casting -1 to a size_t.

Other than that, r=me.

[Darin Adler]

(In reply to comment #3)
> You don't need to cast the result of a void* memory allocation, but it seems to
> be our style to do so. This should be a C++ style cast.

Cameron, I think you're mistaken. You do have to cast void* in C++, even though you don't in plain C because it's a "universal donor" in C.

[Cameron Zwarich (cpst)]

(In reply to comment #4)
> (In reply to comment #3)
> > You don't need to cast the result of a void* memory allocation, but it seems to
> > be our style to do so. This should be a C++ style cast.
> 
> Cameron, I think you're mistaken. You do have to cast void* in C++, even though
> you don't in plain C because it's a "universal donor" in C.

Yeah, I am not sure why I said that. Of course you need to cast in C++.

[Oliver Hunt]

Landed with changes requested by Sam and Cameron

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	JavaScriptCore/ChangeLog
	M	JavaScriptCore/JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCore.vcproj
	M	JavaScriptCore/JavaScriptCore.vcproj/jsc/jsc.vcproj
	M	JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
	M	JavaScriptCore/assembler/AssemblerBuffer.h
	M	JavaScriptCore/assembler/MacroAssembler.h
	M	JavaScriptCore/assembler/X86Assembler.h
	M	JavaScriptCore/bytecode/CodeBlock.cpp
	M	JavaScriptCore/bytecode/CodeBlock.h
	M	JavaScriptCore/bytecode/Instruction.h
	M	JavaScriptCore/interpreter/Interpreter.cpp
	M	JavaScriptCore/interpreter/Interpreter.h
	A	JavaScriptCore/jit/ExecutableAllocator.cpp
	A	JavaScriptCore/jit/ExecutableAllocator.h
	A	JavaScriptCore/jit/ExecutableAllocatorMMAP.cpp
	A	JavaScriptCore/jit/ExecutableAllocatorWin.cpp
	M	JavaScriptCore/jit/JIT.cpp
	M	JavaScriptCore/jit/JIT.h
	M	JavaScriptCore/jit/JITPropertyAccess.cpp
	M	JavaScriptCore/parser/Nodes.cpp
	M	JavaScriptCore/runtime/JSGlobalData.h
	M	JavaScriptCore/runtime/RegExp.cpp
	M	JavaScriptCore/runtime/RegExp.h
	M	JavaScriptCore/runtime/RegExpConstructor.cpp
	M	JavaScriptCore/runtime/RegExpPrototype.cpp
	M	JavaScriptCore/runtime/StringPrototype.cpp
	M	JavaScriptCore/wrec/WREC.cpp
	M	JavaScriptCore/wrec/WRECGenerator.h
	M	JavaScriptCore/wtf/FastMalloc.cpp
	M	JavaScriptCore/wtf/FastMalloc.h
	M	JavaScriptCore/wtf/TCSystemAlloc.cpp
Committed r39083