Bug 21783 - Need more granular control over allocation of executable memory
Summary: Need more granular control over allocation of executable memory
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac Windows XP
: P1 Normal
Assignee: Oliver Hunt
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2008-10-21 23:41 PDT by Oliver Hunt
Modified: 2008-12-07 15:55 PST (History)
3 users (show)

See Also:


Attachments
Patch o' doom (53.96 KB, patch)
2008-12-06 01:24 PST, Oliver Hunt
zwarich: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Hunt 2008-10-21 23:41:23 PDT
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
Comment 1 Mark Rowe (bdash) 2008-10-22 00:36:48 PDT
<rdar://problem/6309878>
Comment 2 Oliver Hunt 2008-12-06 01:24:50 PST
Created attachment 25808 [details]
Patch o' doom

First pass at this forsaken thing
Comment 3 Cameron Zwarich (cpst) 2008-12-07 14:04:01 PST
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.
Comment 4 Darin Adler 2008-12-07 15:45:20 PST
(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.
Comment 5 Cameron Zwarich (cpst) 2008-12-07 15:51:18 PST
(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++.
Comment 6 Oliver Hunt 2008-12-07 15:55:40 PST
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