some write barrier fixes #2293

leirocks · 2016-12-28T20:16:56Z

false poisitive on EntryPointInfo->jsMethod after native address reuse in recycler
false poisitive in arena allocator cacheBlockEnd reused in leaf block in recycler (sharing same page allocator)
overload ChangeToAssign in JIT code, check all posible assign for write barrier
missing barriers in various location

+ minor fixes to JavascriptOperatorA.S

In addition to Debugger support, this PR also brings the required base for compiling code with script profiler.

…es a union in a way that allows a ParseableFunctionInfo to be accessed as though it were a FunctionBody. This led to memory corruption when redeferral changed the meaning of a flag that was meant to protect the accesses. Fixed by removing the union and the flag and using IsFunctionBody/GetFunctionBody to guard against illegal access.

…Works around a parser anomaly that appeared with re-deferral. I'll keep investigating the cause of the anomaly.

Add annotation in CodeGenNumberAllocator.cpp to suppress sal warning. Restructure code in Encoder.cpp, the two function calls were throwing off the analyzer.

Merge pull request chakra-core#2273 from pleath:startbindfunction ByteCodeGenerator::StartBindFunction uses a union in a way that allows a ParseableFunctionInfo to be accessed as though it were a FunctionBody. This led to memory corruption when redeferral changed the meaning of a flag that was meant to protect the accesses. Fixed by removing the union and the flag and using IsFunctionBody/GetFunctionBody to guard against illegal access.

…n fix Merge pull request chakra-core#2273 from pleath:startbindfunction ByteCodeGenerator::StartBindFunction uses a union in a way that allows a ParseableFunctionInfo to be accessed as though it were a FunctionBody. This led to memory corruption when redeferral changed the meaning of a flag that was meant to protect the accesses. Fixed by removing the union and the flag and using IsFunctionBody/GetFunctionBody to guard against illegal access.

Merge pull request chakra-core#2279 from meg-gupta:fixsal Add annotation in CodeGenNumberAllocator.cpp to suppress sal warning. Restructure code in Encoder.cpp, the two function calls were throwing off the analyzer.

…tub for a non-deferred function. Merge pull request chakra-core#2225 from pleath:9871933 Works around a parser anomaly that appeared with re-deferral. I'll keep investigating the cause of the anomaly.

… a deferred stub for a non-deferred function. Merge pull request chakra-core#2225 from pleath:9871933 Works around a parser anomaly that appeared with re-deferral. I'll keep investigating the cause of the anomaly.

Merge pull request chakra-core#2056 from obastemur:debug In addition to Debugger support, this PR also brings the required base for compiling code with script profiler. Fixes chakra-core#2058

…ompiler direction Merge pull request chakra-core#2211 from obastemur:fix_pie + minor fixes to JavascriptOperatorA.S

Merge pull request chakra-core#2272 from obastemur:fix_asmjs_crash

# Conflicts: # build.sh # lib/Runtime/Base/FunctionBody.h

leirocks · 2016-12-28T20:30:11Z

@dotnet-bot test Linux tests please

leirocks · 2016-12-28T20:43:37Z

@dotnet-bot test Linux tests please

leirocks · 2016-12-28T20:51:19Z

@dotnet-bot test Linux tests please

leirocks · 2016-12-28T23:14:32Z

@dotnet-bot test Linux tests please

leirocks · 2016-12-28T23:55:32Z

@dotnet-bot test Linux tests please

leirocks · 2016-12-30T23:30:05Z

@dotnet-bot test Linux tests please

leirocks · 2016-12-31T00:30:46Z

@dotnet-bot test Linux tests please

leirocks · 2016-12-31T00:55:02Z

@dotnet-bot test Linux tests please

leirocks · 2016-12-31T01:16:39Z

@dotnet-bot test OSX daily_osx_osx_test_static please

…e barrier) and some other fix for no write barrier build on windows

looks there's a bug in llvm lto: dlopen() failed; dlerror says '/home/ddccstemp/swb/lto/libChakraCore.so: undefined symbol: _ZTHN6Memory24MemoryOperationLastError14MemOpLastErrorE' this symbol should have been removed after optimization. manually remove here

leirocks · 2017-01-05T23:45:14Z

@dotnet-bot test Linux tests please

leirocks · 2017-01-05T23:49:55Z

@dotnet-bot test Linux tests please

leirocks · 2017-01-06T00:23:00Z

@dotnet-bot test Linux tests please

for some struct like InterpreterStackFrame it's hard to add write barrier annotation. let the recycler aware of such memory, set barrier before rescanning(assuming it's always dirty) currently applied this to generator function feature

while debugger attaching, it deletes current NativeCodeGenerator and free all the native address. then these address can be easily reused by recycler also add more tags on the functionBody to prevent the bit fields becoming pointer like value Record write barrier bit clearing for debugging purpose

leirocks · 2017-01-06T00:53:29Z

@dotnet-bot test Linux tests please

jianchun · 2017-01-10T00:35:44Z

lib/Runtime/Base/FunctionBody.cpp

 #endif
 if (validationCookie == currentCookie)
 {
+ if (this->jsMethod == reinterpret_cast<Js::JavascriptMethod>(this->GetNativeAddress()))


if (this->jsMethod == reinterpret_castJs::JavascriptMethod(this->GetNativeAddress())) [](start = 20, length = 87)

When "jsMethod != ...", what would "jsMethod" contain, could it still create other false positives? Is it ok to always do this without checking "jsMethod != ..."?

jianchun · 2017-01-10T00:43:23Z

test/runtests.py

 if binary == None:
 if sys.platform == 'win32':
- binary = 'Build/VcBuild/bin/{}_{}/ch.exe'.format(arch, flavor)
+ binary = 'Build\\VcBuild\\bin\\{}_{}\\ch.exe'.format(arch, flavor)


Build\VcBuild\bin\{}_{}\ch.exe [](start = 18, length = 34)

Just curious, is this change necessary? I've been running it on Windows and "/" worked.

if you copy the command line with /, ttt won't work...

jianchun · 2017-01-10T00:54:20Z

lib/Runtime/Base/FunctionBody.h

 FieldWithBarrier(bool) hasExecutionDynamicProfileInfo : 1;
+#ifdef _M_X64_OR_ARM64
+ FieldWithBarrier(bool) m_tag : 1; // Used to tag the low bit to prevent possible GC false references
+#endif


Why moving "m_tag"... is it because x64 difference with x86? If so would this make x86 worse?

Also this should be a build break on x86, as now "m_tag" only declared on x64 but initialized on both. (I'm reading in commit order, this could be fixed in later commit.)

yeah, in later commit, I carefully added tag for every 4 bytes. and this won't be build break either, for x86 m_tag is defined in another .h file :)

jianchun · 2017-01-10T00:59:36Z

bin/ch/ch.cpp

 HRESULT ExecuteTestWithMemoryCheck(char* fileName)
 {
 HRESULT hr = E_FAIL;
+#ifdef _WIN32 // looks on linux it always leak ThreadContextTLSEntry since there's no DllMain


Recall Oguz did some change in master, not sure if resolves this.

seems not, hit assertions in linux test

jianchun · 2017-01-10T01:15:56Z

Do you remove this because it overlaps with the other constructor? If so wondering if keeping this constructor makes more sense?

Refers to: lib/Runtime/Base/FunctionBody.h:3702 in 50b4d1a. [](commit_id = 50b4d1a, deletion_comment = True)

leirocks · 2017-01-10T05:05:54Z

yes, it's a duplicate if global write barrier off. and I kept the plain one because many other places we are using this pattern and do the conversion inside function body

In reply to: 271458604 [](ancestors = 271458604)

Refers to: lib/Runtime/Base/FunctionBody.h:3702 in 50b4d1a. [](commit_id = 50b4d1a, deletion_comment = True)

jianchun · 2017-01-10T18:34:33Z

lib/Runtime/SerializableFunctionFields.h

 DECLARE_SERIALIZABLE_FIELD(ProfileId, profiledReturnTypeCount, UInt16);
 DECLARE_SERIALIZABLE_FIELD(ProfileId, profiledSlotCount, UInt16);
 DECLARE_SERIALIZABLE_ACCESSOR_FIELD(uint, LoopCount, RegSlot);
+ DECLARE_SERIALIZABLE_FIELD(bool, m_tag31, Bool); // Used to tag the low bit to prevent possible GC false references


DECLARE_SERIALIZABLE_FIELD(bool, m_tag31, Bool); [](start = 4, length = 48)

Are the "tag" fields being serialized? Is that the reason your change affected bc.h? If so can we avoid serializing the tag fields?

yes, it will be serialized. it's one byte more space in the bytecode. if we choose to not serializing it but add tag here, we might need to change the serializing logic. I didn't spent time on investigate how much work to do though

In reply to: 95427632 [](ancestors = 95427632)

Should be very easy to do. Add a new macro DECLARE_TAG_FIELD in this file which by default does nothing. In FunctionBody.h define it to declare a tag field. Serializer does not see it so no impact.

In reply to: 95429096 [](ancestors = 95429096,95427632)

jianchun · 2017-01-10T18:46:32Z

lib/Runtime/Library/JavascriptLibrary.cpp

+#endif
+ {
+ return RecyclerNew(this->GetRecycler(), JavascriptGenerator, generatorType, args, scriptFunction);
+ }


Consider moving this into a function JavascriptGenerator::New (and make constructor private) so that the logic at the same place.

good idea, let me make the change in next commit.

In reply to: 95430032 [](ancestors = 95430032)

jianchun · 2017-01-10T18:59:20Z

Merge pull request chakra-core#2293 from leirocks:wb3 - false poisitive on EntryPointInfo->jsMethod after native address reuse in recycler - false poisitive in arena allocator cacheBlockEnd reused in leaf block in recycler (sharing same page allocator) - overload ChangeToAssign in JIT code, check all posible assign for write barrier - missing barriers in various location

obastemur and others added 17 commits December 22, 2016 13:14

xplat: fix position independent compiler direction

8335f4c

+ minor fixes to JavascriptOperatorA.S

xplat: fix try-finally issue

f462aa7

xplat: Debugger Support

649f3d2

In addition to Debugger support, this PR also brings the required base for compiling code with script profiler.

Don't attempt to create a deferred stub for a non-deferred function. …

14a02ad

…Works around a parser anomaly that appeared with re-deferral. I'll keep investigating the cause of the anomaly.

Fix sal warnings

5961803

Add annotation in CodeGenNumberAllocator.cpp to suppress sal warning. Restructure code in Encoder.cpp, the two function calls were throwing off the analyzer.

[MERGE chakra-core#2279 @meg-gupta] Fix sal warnings

a2dff3c

Merge pull request chakra-core#2279 from meg-gupta:fixsal Add annotation in CodeGenNumberAllocator.cpp to suppress sal warning. Restructure code in Encoder.cpp, the two function calls were throwing off the analyzer.

[1.4>master] [MERGE chakra-core#2279 @meg-gupta] Fix sal warnings

0c86792

Merge pull request chakra-core#2279 from meg-gupta:fixsal Add annotation in CodeGenNumberAllocator.cpp to suppress sal warning. Restructure code in Encoder.cpp, the two function calls were throwing off the analyzer.

[MERGE chakra-core#2225 @pleath] Don't attempt to create a deferred s…

bc52eb0

…tub for a non-deferred function. Merge pull request chakra-core#2225 from pleath:9871933 Works around a parser anomaly that appeared with re-deferral. I'll keep investigating the cause of the anomaly.

asmjs: anonymous function crash

5449f72

[MERGE chakra-core#2056 @obastemur] xplat: Debugger Support

e5ef2d1

Merge pull request chakra-core#2056 from obastemur:debug In addition to Debugger support, this PR also brings the required base for compiling code with script profiler. Fixes chakra-core#2058

[MERGE chakra-core#2211 @obastemur] xplat: fix position independent c…

41779e7

…ompiler direction Merge pull request chakra-core#2211 from obastemur:fix_pie + minor fixes to JavascriptOperatorA.S

[MERGE chakra-core#2272 @obastemur] asmjs: anonymous function crash

62f9668

Merge pull request chakra-core#2272 from obastemur:fix_asmjs_crash

Merge branch 'master' into swb

2a34df5

# Conflicts: # build.sh # lib/Runtime/Base/FunctionBody.h

msftclas added the cla-already-signed label Dec 28, 2016

leirocks force-pushed the wb3 branch 2 times, most recently from a562259 to 21bc6f2 Compare December 28, 2016 20:26

leirocks force-pushed the wb3 branch from 21bc6f2 to ecc69ba Compare December 28, 2016 20:43

leirocks force-pushed the wb3 branch from ecc69ba to f4915ff Compare December 28, 2016 20:51

leirocks force-pushed the wb3 branch from f4915ff to 525a2e1 Compare December 28, 2016 23:13

leirocks force-pushed the wb3 branch from 525a2e1 to ce04185 Compare December 28, 2016 23:54

ttd fix for recyclerConcurrentStress

83b92a7

leirocks force-pushed the wb3 branch from 897f4af to 514543d Compare December 30, 2016 23:29

leirocks force-pushed the wb3 branch from 514543d to 559b033 Compare December 31, 2016 00:30

try to enable recycler track on Linux

50b4d1a

leirocks force-pushed the wb3 branch from 559b033 to 50b4d1a Compare December 31, 2016 00:54

leirocks added 2 commits January 3, 2017 14:00

fix for the special constructor for functionBody (which enforced writ…

578f5b9

…e barrier) and some other fix for no write barrier build on windows

leirocks force-pushed the wb3 branch from 566210d to 65bf2d9 Compare January 5, 2017 23:48

leirocks force-pushed the wb3 branch from 65bf2d9 to 88d8c43 Compare January 6, 2017 00:22

leirocks added 2 commits January 5, 2017 16:52

leirocks force-pushed the wb3 branch from 88d8c43 to ee6913a Compare January 6, 2017 00:53

jianchun reviewed Jan 10, 2017

View reviewed changes

jianchun merged commit ee6913a into chakra-core:swb Jan 10, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

some write barrier fixes #2293

some write barrier fixes #2293

Uh oh!

leirocks commented Dec 28, 2016

leirocks commented Dec 28, 2016

leirocks commented Dec 28, 2016

leirocks commented Dec 28, 2016

leirocks commented Dec 28, 2016

leirocks commented Dec 28, 2016

leirocks commented Dec 30, 2016

leirocks commented Dec 31, 2016

leirocks commented Dec 31, 2016

leirocks commented Dec 31, 2016

leirocks commented Jan 5, 2017

leirocks commented Jan 5, 2017

leirocks commented Jan 6, 2017

leirocks commented Jan 6, 2017

jianchun Jan 10, 2017

jianchun Jan 10, 2017

leirocks Jan 10, 2017

jianchun Jan 10, 2017

leirocks Jan 10, 2017

jianchun Jan 10, 2017

leirocks Jan 10, 2017

jianchun commented Jan 10, 2017

leirocks commented Jan 10, 2017

jianchun Jan 10, 2017

leirocks Jan 10, 2017

jianchun Jan 10, 2017

jianchun Jan 10, 2017

leirocks Jan 10, 2017

jianchun commented Jan 10, 2017

Labels

5 participants

some write barrier fixes #2293

some write barrier fixes #2293

Uh oh!

Conversation

leirocks commented Dec 28, 2016

leirocks commented Dec 28, 2016

leirocks commented Dec 28, 2016

leirocks commented Dec 28, 2016

leirocks commented Dec 28, 2016

leirocks commented Dec 28, 2016

leirocks commented Dec 30, 2016

leirocks commented Dec 31, 2016

leirocks commented Dec 31, 2016

leirocks commented Dec 31, 2016

leirocks commented Jan 5, 2017

leirocks commented Jan 5, 2017

leirocks commented Jan 6, 2017

leirocks commented Jan 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jianchun commented Jan 10, 2017

leirocks commented Jan 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jianchun commented Jan 10, 2017

Labels

5 participants