WASM Xplat: Linux #1727

arunetm-zz · 2016-10-12T06:04:42Z

Enabling Webassembly to build with Linux

obastemur

I don't see any place, this module is part of resulting binary. Is that intended?

arunetm-zz · 2016-10-12T16:42:37Z

@Cellule @MikeHolman please review.

Cellule · 2016-10-12T18:23:02Z

lib/WasmReader/WasmBinaryReader.cpp

-    mbstowcs_s(&convertedChars, buf, idSize + 1, sectionName, _TRUNCATE);
-    buf[idSize] = 0;
-    TRACE_WASM_SECTION(_u("Section Header: %s, length = %u (0x%x)"), buf, sectionSize, sectionSize);
+    TRACE_WASM_SECTION(_u("Section Header: %s, length = %u (0x%x)"), sectionName, sectionSize, sectionSize);


sectionName is a char* does it work in with a printf function using a char16 format string ?

I'm not sure if %S will work here to print char string. If that doesn't, try use utf8::NarrowToWide

Had changed sectionName to wchar_t to avoid the use of mbstowcs_s in linux to get around a method not defined error. Will change this back to char16* and look for another workaround to make it compile in xplat.

Cellule · 2016-10-12T18:24:32Z

lib/WasmReader/WasmSection.h

@@ -27,8 +27,8 @@ namespace Wasm
    {
        SectionFlag flag;
        SectionCode precedent;
-        char16* name;
-        char* id;
+        const wchar_t* name;


I thought the point of char16 was for cross plat

+1. Please try avoid wchar_t.

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

Will change this back. The use of "mbstowcs_s" was giving pain in Linux, will look for a workaround.

Cellule · 2016-10-12T18:27:55Z

@dotnet-bot test linux tests please

Cellule · 2016-10-12T18:31:53Z

@obastemur I'm sorry for my ignorance, but what does it mean for a module to be part of the resulting binary or not?

jianchun · 2016-10-12T18:39:42Z

lib/WasmReader/CMakeLists.txt

@@ -0,0 +1,20 @@
+add_library (Chakra.WasmReader OBJECT
+


nit: unneeded extra blank line

will remove.

jianchun · 2016-10-12T18:42:43Z

lib/Runtime/ByteCode/OpLayoutsAsmJs.h

 #define LAYOUT_TYPE_WMS_REG2(layout, t0, t1) \
    template <typename SizePolicy> struct OpLayoutT_##layout\
    {\
-        t0##LayoutType LAYOUT_PREFIX_##t0()0;\
-        t1##LayoutType LAYOUT_PREFIX_##t1()1;\
+        t0##LayoutType LAYOUT_FIELDS_DEF(LAYOUT_PREFIX_##t0(), 0);\


LAYOUT_FIELDS_DEF(LAYOUT_PREFIX_##t0(), 0);\ [](start = 23, length = 44)

Is this to have () in a field name (weird, why do we want that)? From the change I assume directly ##0 doesn't work, right? #Resolved

() is part of the macro name and it is not going to be part of the actual field name. The tricky part was to have the number to be appended to the end of the filed name to avoid naming conflicts.
##0 does not work as ## expects strings as operands.

jianchun · 2016-10-12T18:49:55Z

#ifdef ENABLE_WASM

Is ENABLE_WASM turned on Linux?

Refers to: lib/WasmReader/WasmSection.h:8 in de4503e. [](commit_id = de4503e, deletion_comment = False)

jianchun · 2016-10-12T18:53:04Z

    if (!memcmp(SectionInfo::All[i].id, sectionName, idSize))

Does "sectionId" sound a better name for this variable (I was confused and thought this was the same as sectionInfo.name)?

Refers to: lib/WasmReader/WasmBinaryReader.cpp:167 in de4503e. [](commit_id = de4503e, deletion_comment = False)

jianchun · 2016-10-12T18:57:29Z

@obastemur I'm sorry for my ignorance, but what does it mean for a module to be part of the resulting binary or not?

The new Chakra.WasmReader lib is not linked to anything. Kind of related to my other question -- is it enabled on Linux? Suppose it would link to ch and Jsrt.lib?

arunetm-zz · 2016-10-12T19:16:45Z

@jianchun, ENABLE_WASM is turned on for Linux. Its on for all debug/test builds.
@Cellule, can you please verify whether this is the right thing to do?

arunetm-zz · 2016-10-12T19:18:39Z

@obastemur, @jianchun Forgive my ignorance, I missed the step to link Cahkra.WasmReader lib for Linux. Will make this change and update the PR.
Thanks!

Cellule · 2016-10-12T19:45:19Z

@arunetm We don't need to enable WebAssembly on Linux right now. I don't see the harm, but regardless it is intended to be enable only in debug and test builds at the moment since it is still a WIP

MikeHolman · 2016-10-12T19:55:31Z

@Cellule I think it is useful to have on Linux, if for no other reason than to be able to fuzz with AFL.

Cellule · 2016-10-13T04:49:54Z

@MikeHolman Yes I believe it would be good to have, but what I am wondering is, does it have to be a blocker to merge WebAssembly to master ?

Cellule · 2016-10-13T21:46:11Z

Any update on this ?

arunetm-zz · 2016-10-13T23:58:59Z

@Cellule Currently fixing a linker error and looking for a workaround for mbstowcs_s.
Will update the PR soon I get to a solution.

arunetm-zz · 2016-10-14T06:08:49Z

The linker fails to find WasmLibrary::WasmDeferredParseExternalThunk defined in Language/amd64/amd64_Thunks.asm (called from Library/WasmLibrary.cpp). CMakeLists.txt includes only Language/amd64/amd64_Thunks.S and we don't have the WasmDeferredParseExternalThunk defined in it (hence this issue)
Forgive my ignorance of cmake, but adding amd64_Thunks.asm to CMakeLists.txt doesn't seem to recognize the format and it leads to many failures.

@jianchun. This is a long shot question but, is it possible to allow CMake to recognize the .asm format and include Language/amd64/amd64_Thunks.asm for the build?

@Cellule Is there a reason why we have WasmDeferredParseExternalThunk defined in Languages/amd64/amd64_Thunks.asm instead of Library/amd64/JavaScriptFunction.asm since its being called from Library/WasmLibrary.cpp only? Should we move it?
Also to have amd64_Thunks.S (gnu compliant) implement WasmDeferredParseExternalThunk, does it need to be written again from scratch?
thanks in advance.

jianchun · 2016-10-14T17:06:32Z

@arunetm WasmDeferredParseExternalThunk needs to be rewritten in amd64_Thunks.S. You can see that by comparing amd64_Thunks.asm and amd64_Thunks.S.

arunetm-zz · 2016-10-14T19:48:36Z

@Cellule @MikeHolman. I'm starting to rewrite the WasmDeferredParseExternalThunk inside amd64_Thunks.S (or any other possible missing pieces) to make it available for xplat builds. Is this path beneficial to do this at this point or should we disable WebAssembly in Linux in preparation to the master integration?

MikeHolman · 2016-10-14T19:56:36Z

@arunetm I think it's beneficial to do, but for the merge to master let's just disable on x-plat.

Asmjs fixes (Builds in Linux with no ENABLE_WASM)

402b635

msftclas added the cla-already-signed label Oct 12, 2016

WASM Xplat: Linux

de4503e

Enabling Webassembly to build with Linux

arunetm-zz force-pushed the b-arpuru/WASM-Linux branch from 53397b4 to de4503e Compare October 12, 2016 06:09

obastemur suggested changes Oct 12, 2016

View reviewed changes

Cellule reviewed Oct 12, 2016

View reviewed changes

jianchun reviewed Oct 12, 2016

View reviewed changes

Cellule mentioned this pull request Oct 15, 2016

Wasm: Xplat stage #1750

Merged

Cellule merged commit de4503e into chakra-core:WebAssembly_Stage Oct 15, 2016

snyk-bot mentioned this pull request Nov 18, 2020

[Snyk] Security upgrade yargs from 4.8.1 to 16.0.0 saurabharch/ChakraCore#3

Open

WASM Xplat: Linux #1727

WASM Xplat: Linux #1727

Uh oh!

Conversation

arunetm-zz commented Oct 12, 2016

Uh oh!

obastemur left a comment

Choose a reason for hiding this comment

Uh oh!

arunetm-zz commented Oct 12, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Cellule commented Oct 12, 2016

Uh oh!

Cellule commented Oct 12, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jianchun Oct 12, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jianchun commented Oct 12, 2016

Uh oh!

jianchun commented Oct 12, 2016

Uh oh!

jianchun commented Oct 12, 2016

Uh oh!

arunetm-zz commented Oct 12, 2016

Uh oh!

arunetm-zz commented Oct 12, 2016

Uh oh!

Cellule commented Oct 12, 2016

Uh oh!

MikeHolman commented Oct 12, 2016

Uh oh!

Cellule commented Oct 13, 2016

Uh oh!

Cellule commented Oct 13, 2016

Uh oh!

arunetm-zz commented Oct 13, 2016

Uh oh!

arunetm-zz commented Oct 14, 2016

Uh oh!

jianchun commented Oct 14, 2016

Uh oh!

arunetm-zz commented Oct 14, 2016

Uh oh!

MikeHolman commented Oct 14, 2016

Uh oh!

Uh oh!

jianchun Oct 12, 2016 •

edited

Loading