Revert the original fix to 5424307 #2

pleath · 2016-01-05T21:57:11Z

Revert the original fix to 5424307 (difference in error thrown in deferred and non-deferred case of assignment to function result) as it was causing more problems than it fixed. Will come up with a proper fix for the original bug.

msftclas · 2016-01-05T21:57:14Z

Hi @pleath, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

It looks like you're a Microsoft contributor (Paul Leathers). If you're full-time, we DON'T require a Contribution License Agreement. If you are a vendor, please DO sign the electronic Contribution License Agreement. It will take 2 minutes and there's no faxing! https://cla.microsoft.com.

TTYL, MSBOT;

pleath · 2016-01-06T19:53:22Z

@abchatra, I think you reviewed the original change. Can you review the undo? (Thanks.)

…erred and non-deferred case of assignment to function result) as it was causing more problems than it fixed. Will come up with a proper fix for the original bug.

Merge pull request #305 from pleath:released/1602 #1: Fix bugs with non-canonical NaNs in asm.js In asm.js, we don't canonicalize NaNs. We don't have vars in asm.js, so this is ok. But we DO need to make sure to check for NaN if we are creating a JavascriptNumber from it. However, we were not doing this, which was an issue for argouts in asm.js->js calls as well as return values from asm.js. It was also an issue in one helper call on x64, where we were converting to var before making the call. This was being done because there was no support for x64 calling convention. But with asm.js, I implemented that support, so I updated the helper call to use the standard API. Other places were also hacking around this limitation, so I updated them as well. #2: add check against length for memset #3: Issue: There was a code path where we protect pages with PAGE_EXECUTE without the PAGE_TARGETS_NO_UPDATE flag, which will control all the CFG bits in the page. Fix: Fixed and ensured that we don't use PAGE_EXECUTE directly in the virtual protect API. Instead #defines are used. Cleaned up some complex code path in CustomHeap Protect Allocation mechanism. Removed isReadWrite flag from PageAllocation class, as it is not required. Earlier, we were also letting RW pages to be added to the bucket list of pages in custom heap, which lead the hacker to add his own RW page to the list. Now it is made sure that only RX pages go into the bucket list. Tests: Unit tests passed. Exprgen, Web crawler - Reran the tests and they look fine..

…tate bytecode generation. Setup ModuleRecord list in JavascriptLibrary to allow ModuleId lookup. Setup ModuleId for bytecode generation. When a new modulerecord is created, it is added to a list (moduleRecordList) in JavascriptLibrary. the index is the moduleID. There is a dependent childModuleRecord dictionary with specifier as key and modulerecord as value, this way we can generate the bytecode using the moduleId. All the local exports are stored in the localExportSlots, used in bytecode generator. Temporary disable anonymous function/class export. Need to fix the anonymous function binding.

Merge pull request #363 from pr/yongqu/module module runtime #2: merge change with parser; setup localexport slot to facilitate bytecode generation. Setup ModuleRecord list in JavascriptLibrary to allow ModuleId lookup. Setup ModuleId for bytecode generation. When a new modulerecord is created, it is added to a list (moduleRecordList) in JavascriptLibrary. the index is the moduleID. There is a dependent childModuleRecord dictionary with specifier as key and modulerecord as value, this way we can generate the bytecode using the moduleId. All the local exports are stored in the localExportSlots, used in bytecode generator. Temporary disable anonymous function/class export. Need to fix the anonymous function binding.

- Avoid pinning opnds to byte-addressable reg. Copy to new TyInt8 sym instead. - Removed ununsed IRBuilder/BytecodeDumper functions. - SccLiveness will process dst of SHUFPS/D as a src - Minor nit fixes in LowererMDSharedSimd128

…g ops. - Added Int32x4 min/max, shiftRight/LeftByScalar - For Int16x8, Uint32x4, Uint16x8, Uint8x16: * Enabled constructor/check calls, argument passing, returns, globals import, passing values to modules, return value from module. * Added opcodes, layouts, IRBuilders, Bytecode dumps. Implemented interpreter handlers. - Fixes in lowerer and adding new IRTypes to RegAlloc, Bailout records ... - Relaxed shuffle constraint to accept arbitrary in-range lane indices - Added lowerer support for the following ops of Uint32x4, Uint16x8, Uint8x16, and Int16x8: Constructors, replace/extractLane - Added shuffle/swizzle, load/store operations for all types except Int8x16 - Uint32x4/16x8/8x16 and Int16x8 conversion ops in lowerer - Asmjs support for SIMD bool types - Disabling CSE for new opcodes to accomodate the present opcode space for global opt. - Enabled anyTrue/allTrue coercion check, comparison and select ops for all types. - Several bug fixes in arguments passing/return values, and register allocation for bool types. - Fixed Comparison/select tests for Int32x4 and Float32x4. - Removed Float64x2 support from Asm.js front-end. Fixed tests accordingly. - Added comparison unit-tests for new types: Int16x8, and all unsigned ints. Enable AsmJs Lower support - Changed SIMD object, constructors, and their properties to be Configurable and Writeable. - AsmJs linker does a run timecheck for used SIMD ops - Cleaned up obsolete ops: scale, zero, bool, withFlag* and others, modified UTs. - Changed line endings on JavascriptSIMD*.cpp files to CRLF Added Unit-tests, and fixes to ASMJS link. Added Min/MaxNum. Minor fixes to UTs New SIMD Bool opearions. 1. Adding Constructor, splat, Extract/Replace lane for all SIMD bools. 2. Cleaning up D2 opcodes to make space for the new opcodes. 3. Adding tests. Fix code analysis failure in release build CR Fixes #1 Adding Neg operation for Uint32x4, Uint16x8, Uint8x16. Neg operation for unsigned types in Library, Interpreter and JIT. CR Fixes chakra-core#2: - Avoid pinning opnds to byte-addressable reg. Copy to new TyInt8 sym instead. - Removed ununsed IRBuilder/BytecodeDumper functions. - SccLiveness will process dst of SHUFPS/D as a src - Minor nit fixes in LowererMDSharedSimd128 Fix copyright headers

This change does the following : - Eliminates stack probe for leaf functions with small stack footprint - Eliminates arg saves on stack for a leaf function with no arg usage - Eliminate redundant null store on stack ----------------------------------------------------------------------------- Empty function before : ----------------------------------------------------------------------------- Function empty ( (chakra-core#1.1), chakra-core#2) Instr Count:38 FunctionEntry (rax).i64 = MOV 0xXXXXXXXX (&StackLimit).u64 (rax).i64 = MOV [(rax).i64].i64 (rax).i64 = ADD (rax).i64, 0x000000001BD0.u64 JO $L4 CMP (rsp).i64, (rax).i64 JLE $L4 NOP 4 (0x4).i8 NOP 2 (0x2).i8 arg5(s8)<32>.i64 = MOV (r9).i64 arg4(s7)<24>.i64 = MOV (r8).i64 arg3(s6)<16>.i64 = MOV (rdx).i64 arg2(s5)<8>.i64 = MOV (rcx).i64 PrologStart PUSH (rbp).i64 (rbp).i64 = MOV (rsp).i64 (rsp).i64 = SUB (rsp).i64, 64 (0x40).i32 PrologEnd (rax).u32 = XOR (rax).u32, (rax).u32 s4<-8>.i64 = MOV (rax).i64 s3(rax).u64 = MOV 0xXXXXXXXX (&CallCount).u64 CMP [s3(rax).u64].u8, 255 (0xFF).u8 JEQ $L3 [s3(rax).u64].u8 = INC [s3(rax).u64].u8 $L3: s0(rax)[Undefined].var = MOV 0xXXXXXXXX (undefined)[Undefined].var Line 7: } Col 1: ^ StatementBoundary #0 StatementBoundary #-1 (rsp).i64 = MOV (rbp).i64 (rbp).i64 = POP RET 0 (0x0).i32, (rax).i64 FunctionExit $L4: [helper] (rdx).i64 = MOV 0xXXXXXXXX (ScriptContext).u64 (rcx).i64 = MOV 0x000000001BD0.u64 (rax).i64 = MOV ProbeCurrentStack.u64 JMP (rax).i64 StatementBoundary #- ----------------------------------------------------------------------------- ----------------------------------------------------------------------------- Empty function after : ----------------------------------------------------------------------------- Function empty ( (chakra-core#1.1), chakra-core#2) Instr Count:18 FunctionEntry PrologStart PUSH (rbp).i64 (rbp).i64 = MOV (rsp).i64 (rsp).i64 = SUB (rsp).i64, 32 (0x20).i32 PrologEnd s3(rax).u64 = MOV 0xXXXXXXXX (&CallCount).u64 CMP [s3(rax).u64].u8, 255 (0xFF).u8 JEQ $L3 [s3(rax).u64].u8 = INC [s3(rax).u64].u8 $L3: s0(rax)[Undefined].var = MOV 0xXXXXXXXX (undefined)[Undefined].var Line 7: } Col 1: ^ StatementBoundary #0 StatementBoundary #-1 (rsp).i64 = MOV (rbp).i64 (rbp).i64 = POP RET 0 (0x0).i32, (rax).i64 FunctionExit -----------------------------------------------------------------------------

1. Pinned object fakeGlobalFuncForUndefer (as well as profileInfoList in test/debug build) reference to javascriptLibrary -- directly or indirectly, these are rely on ScriptContext::Close() to unpin. 2. javascriptLibrary has a reference to JsrtContext 3. JsrtContext is pinned while setting to current thread, and unpinned when getting out of current thread 4. if user code didn't explicited pin JsrtContext (in following POC), at this stage it should be disposed in next GC, and hence call ScriptContext::Close() 5. the disposal in chakra-core#4 didn't because JsrtContext is reachable through fakeGlobalFuncForUndefer->javascriptLibrary->JsrtContext(chakra-core#2), so the whole graph is leaked 6. when there's external call to JsDisposeRuntime, it will directly dispose JsrtContext, and then ScriptContext::Close, unpin fakeGlobalFuncForUndefer then everything is collectable the POC: ```c++ JsRuntimeHandle runtime; unsigned currentSourceContext = 0; JsCreateRuntime(JsRuntimeAttributeNone, nullptr, &runtime); auto runJob = [&](wstring script, int i) { { JsValueRef result; JsContextRef context; JsCreateContext(runtime, &context); JsSetCurrentContext(context); JsRunScript(script.c_str(), currentSourceContext++, L"", &result); JsSetCurrentContext(JS_INVALID_REFERENCE); context = nullptr; result = nullptr; } if (i % 5 == 0) { JsCollectGarbage(runtime); // JsrtContext in above scope should be collectible at this point, // but the Finalize/Dispose of JsrtContext didn't happen } }; for (int i = 0; i < 100; i++) { runJob(L"(()=>{return \'Hello world!\';})()", i); } printf("JsDisposeRuntime\n"); JsDisposeRuntime(runtime); // all JsrtContext will be collected at this point printf("After JsDisposeRuntime\n"); ``` The fix is, do not pin fakeGlobalFuncForUndefer and profileInfoList. However, there are a lot of code(mostly debugger related code) rely on the leak to do the cleanup. Most of the work is to make sure the cleanup working correctly (without either UAF or leak).

…oogle, Inc.

- Google, Inc. This change completes a previous fix by not caching deep-copied arrays to ensure that only shallow-copied arrays are reused.

Since bailing in happens in the jit'd code, we have to generate code to output the trace. If tracing is enabled, we will fill the array of bail-in symbols in the generator instance (`bailInSymbolsTraceArray`) with their ids and values and finally output them with a call to a runtime helper. `-trace:bailin` together with `-trace:bailout -verbose` can help us easily debug jit'd generators by comparing the values when bailing out for `yield` and bailing in: ``` BailOut: function: func68 ( (chakra-core#1.1), chakra-core#2) offset: #003f Opcode: Yield Kind: BailOutForGeneratorYield BailOut: Register # 0: Not live BailOut: Register # 1: Constant table BailOut: Register # 2: Register r15 16, value: 0x0000023CE132EEA0 (Yield Return Value) BailOut: Register # 3: Register r12 13, value: 0x0001000000000004 BailOut: Return Value: 0x0000023CE132EEA0 BailIn: function: func68 ( (chakra-core#1.1), chakra-core#2) offset: chakra-core#42 BailIn: Register # 3, value: 0x0001000000000004 ``` ``` BailOut: function: func68 ( (chakra-core#1.1), chakra-core#2) offset: #006b Opcode: Yield Kind: BailOutForGeneratorYield BailOut: Register # 0: Not live BailOut: Register # 1: Constant table BailOut: Register # 3: Register r15 16, value: 0x0000023CE133E060 (Yield Return Value) BailOut: Return Value: 0x0000023CE133E060 BailIn: function: func68 ( (chakra-core#1.1), chakra-core#2) offset: #006e BailIn: No symbols reloaded ```

…l-in Merge pull request #6183 from nhat-nguyen:trace Since bailing in happens in the jit'd code, we have to generate code to output the trace. If tracing is enabled, we will fill the array of bail-in symbols in the generator instance (`bailInSymbolsTraceArray`) with their ids and values and finally output them with a call to a runtime helper. `-trace:bailin` together with `-trace:bailout -verbose` can help us easily debug jit'd generators by comparing the values when bailing out for `yield` and bailing in: ``` BailOut: function: func68 ( (#1.1), #2) offset: #003f Opcode: Yield Kind: BailOutForGeneratorYield BailOut: Register # 0: Not live BailOut: Register # 1: Constant table BailOut: Register # 2: Register r15 16, value: 0x0000023CE132EEA0 (Yield Return Value) BailOut: Register # 3: Register r12 13, value: 0x0001000000000004 BailOut: Return Value: 0x0000023CE132EEA0 BailIn: function: func68 ( (#1.1), #2) offset: #42 BailIn: Register # 3, value: 0x0001000000000004 ``` ``` BailOut: function: func68 ( (#1.1), #2) offset: #006b Opcode: Yield Kind: BailOutForGeneratorYield BailOut: Register # 0: Not live BailOut: Register # 1: Constant table BailOut: Register # 3: Register r15 16, value: 0x0000023CE133E060 (Yield Return Value) BailOut: Return Value: 0x0000023CE133E060 BailIn: function: func68 ( (#1.1), #2) offset: #006e BailIn: No symbols reloaded ```

msftclas added the cla-not-required label Jan 5, 2016

pleath assigned abchatra Jan 6, 2016

ianwjhalliday added cla-not-required and removed cla-not-required labels Jan 6, 2016

Revert the original fix to 5424307 (difference in error thrown in def…

a737f00

…erred and non-deferred case of assignment to function result) as it was causing more problems than it fixed. Will come up with a proper fix for the original bug.

chakrabot merged commit a737f00 into chakra-core:master Jan 6, 2016

pleath mentioned this pull request Feb 12, 2016

Updates for 16-02 #305

Merged

Yongqu mentioned this pull request Feb 22, 2016

module runtime #2 #363

Merged

pleath mentioned this pull request Apr 13, 2016

let vs var generates a lot more instructions #800

Open

leirocks mentioned this pull request Feb 24, 2017

Fix leak in JSRT #2592

Merged

djn3m0 mentioned this pull request Mar 16, 2017

Chakra compiled with ASAN not working (in non-static builds) #2694

Closed

KarenHuang2016 mentioned this pull request Mar 31, 2017

Test "NumberFormatOptionsImplSpecific-Win10.js" failed on Windows Server 2012 R2 #2769

Closed

LouisLaf mentioned this pull request Jun 12, 2017

Profile data should get propagated from inliner to inlinees #3140

Closed

litian2025 mentioned this pull request Jun 29, 2017

ChakraCore hello scenario generates segmentation fault on Linux Skylake server #3262

Closed

rajatd mentioned this pull request Jul 26, 2017

Heap overflow in CreateTTDDirectoryAsNeeded #3434

Closed

jackhorton mentioned this pull request Sep 24, 2017

Update build system for more generic include paths #3781

Closed

thomasmo pushed a commit to thomasmo/ChakraCore that referenced this pull request Jan 4, 2018

[CVE-2018-0775] Deferred parsing makes wrong scopes chakra-core#2 - G…

0f736fa

…oogle, Inc.

thomasmo pushed a commit to thomasmo/ChakraCore that referenced this pull request Jan 5, 2018

[CVE-2018-0775] Deferred parsing makes wrong scopes chakra-core#2 - G…

ee5ac64

…oogle, Inc.

zhunki mentioned this pull request Jul 26, 2018

null pointer dereference in EmitBooleanExpression #5532

Closed

crdumoul mentioned this pull request Aug 7, 2018

Seg fault when accessing Context from multiple threads #5577

Closed

ghost mentioned this pull request Mar 21, 2019

chakracore throws an abort trap (or oom crash on edge) with an invalid length to String.prototype.repeat ... maybe just display an error massage? #6042

Closed

bird8693 mentioned this pull request Mar 17, 2021

Segmentation fault #6643

Open

wangyue6761 mentioned this pull request Feb 20, 2022

Segmentation fault #6789

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revert the original fix to 5424307 #2

Revert the original fix to 5424307 #2

pleath commented Jan 5, 2016

msftclas commented Jan 5, 2016

pleath commented Jan 6, 2016

Revert the original fix to 5424307 #2

Revert the original fix to 5424307 #2

Conversation

pleath commented Jan 5, 2016

msftclas commented Jan 5, 2016

pleath commented Jan 6, 2016