Re: [Mono-dev] [PATCH] more support for Google Native Client
Hello! Firstly I must say big thanks to Elijah again. I had successfully run a part of our C# code with Mono on NaCl. But occasionally I'm finding some bugs with global variables in Mono, which value is equal 0x. For example: * assemly_search_hook in assembly.c, * global_codeman in mini.c. I think NaCl corrupts the values of variables during the initialization or during running application. May be somebody had this problem before? -- View this message in context: http://mono.1490590.n4.nabble.com/PATCH-more-support-for-Google-Native-Client-tp3159583p3422375.html Sent from the Mono - Dev mailing list archive at Nabble.com. ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] more support for Google Native Client
Elijah, I had compiled Mono from your patch. Big thanks for your complete answer! -- View this message in context: http://mono.1490590.n4.nabble.com/PATCH-more-support-for-Google-Native-Client-tp3159583p3313930.html Sent from the Mono - Dev mailing list archive at Nabble.com. ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [PATCH] more support for Google Native Client
Hello! I'm trying to compile Mono 2.10 for Nacl at Ubuntu with using Elijah's patch. After executing nacl-runtime-mono.sh I had some error about undefined reference: genmdesc.c:29: undefined reference to `__nacl_suspend_thread_if_needed' Why it happened? Please, help me. I want to compile mono very much. P.S. End of log which I get: /bin/bash ../../libtool --tag=CC --mode=link /home/vladimir/nacl/native_client/toolchain/linux_x86/bin/nacl-gcc -I../../../.. -I../../../../eglib/src -I../../eglib/src-g -D_POSIX_PATH_MAX=256 -DPATH_MAX=256 -fno-strict-aliasing -Wdeclaration-after-statement -g -Wall -Wunused -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Wmissing-prototypes -Wnested-externs -Wpointer-arith -Wno-cast-qual -Wwrite-strings -mno-tls-direct-seg-refs -o genmdesc genmdesc-genmdesc.o genmdesc-helpers.o genmdesc-opcodes.o ../../mono/utils/libmonoutils.la -lm -L../../eglib/src -leglib -lm -lm -lnosys -lg -lpthread libtool: link: /home/vladimir/nacl/native_client/toolchain/linux_x86/bin/nacl-gcc -I../../../.. -I../../../../eglib/src -I../../eglib/src -g -D_POSIX_PATH_MAX=256 -DPATH_MAX=256 -fno-strict-aliasing -Wdeclaration-after-statement -g -Wall -Wunused -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Wmissing-prototypes -Wnested-externs -Wpointer-arith -Wno-cast-qual -Wwrite-strings -mno-tls-direct-seg-refs -o genmdesc genmdesc-genmdesc.o genmdesc-helpers.o genmdesc-opcodes.o ../../mono/utils/.libs/libmonoutils.a -L/home/vladimir/mono/nacl/runtime-build/eglib/src /home/vladimir/mono/nacl/runtime-build/eglib/src/.libs/libeglib.a -lm -lnosys -lg -lpthread /home/vladimir/mono/nacl/runtime-build/eglib/src/.libs/libeglib.a(libeglib_la-gpath.o): In function `monoeg_g_find_program_in_path': /home/vladimir/mono/nacl/runtime-build/eglib/src/../../../../eglib/src/gpath.c:226: warning: the `access' function is not implemented and will always fail /home/vladimir/nacl/native_client/toolchain/linux_x86/bin/../lib/gcc/nacl64/4.4.3/../../../../nacl64/lib/../lib32/libg.a(lib_a-execve.o): In function `execve': execve.c:(.text+0xa): warning: the `_execve' function is not implemented and will always fail /home/vladimir/nacl/native_client/toolchain/linux_x86/bin/../lib/gcc/nacl64/4.4.3/../../../../nacl64/lib/../lib32/libg.a(lib_a-execr.o): In function `_fork_r': execr.c:(.text+0x9c): warning: the `fork' function is not implemented and will always fail /home/vladimir/mono/nacl/runtime-build/eglib/src/.libs/libeglib.a(libeglib_la-gfile-posix.o): In function `monoeg_g_get_current_dir': /home/vladimir/mono/nacl/runtime-build/eglib/src/../../../../eglib/src/gfile-posix.c:158: warning: the `getcwd' function is not implemented and will always fail /home/vladimir/mono/nacl/runtime-build/eglib/src/.libs/libeglib.a(libeglib_la-gmisc-unix.o): In function `get_pw_data': /home/vladimir/mono/nacl/runtime-build/eglib/src/../../../../eglib/src/gmisc-unix.c:97: warning: the `getpwuid_r' function is not implemented and will always fail /home/vladimir/mono/nacl/runtime-build/eglib/src/../../../../eglib/src/gmisc-unix.c:97: warning: the `getuid' function is not implemented and will always fail genmdesc-helpers.o: In function `mono_disassemble_code': /home/vladimir/mono/nacl/runtime-build/mono/mini/../../../../mono/mini/helpers.c:253: warning: the `unlink' function is not implemented and will always fail /home/vladimir/nacl/native_client/toolchain/linux_x86/bin/../lib/gcc/nacl64/4.4.3/../../../../nacl64/lib/../lib32/libg.a(lib_a-execr.o): In function `_wait_r': execr.c:(.text+0x1c): warning: the `wait' function is not implemented and will always fail genmdesc-genmdesc.o: In function `monoeg_strdup': /home/vladimir/mono/nacl/runtime-build/mono/mini/../../../../eglib/src/glib.h:148: undefined reference to `__nacl_suspend_thread_if_needed' genmdesc-genmdesc.o: In function `inst_name': /home/vladimir/mono/nacl/runtime-build/mono/mini/../../../../mono/mini/genmdesc.c:29: undefined reference to `__nacl_suspend_thread_if_needed' /home/vladimir/mono/nacl/runtime-build/mono/mini/../../../../mono/mini/genmdesc.c:34: undefined reference to `__nacl_suspend_thread_if_needed' /home/vladimir/mono/nacl/runtime-build/mono/mini/../../../../mono/mini/genmdesc.c:34: undefined reference to `__nacl_suspend_thread_if_needed' genmdesc-genmdesc.o: In function `load_file': /home/vladimir/mono/nacl/runtime-build/mono/mini/../../../../mono/mini/genmdesc.c:53: undefined reference to `__nacl_suspend_thread_if_needed' genmdesc-genmdesc.o:/home/vladimir/mono/nacl/runtime-build/mono/mini/../../../../mono/mini/genmdesc.c:62: more undefined references to `__nacl_suspend_thread_if_needed' follow collect2: ld returned 1 exit status make[3]: *** [genmdesc] Ошибка 1 make[3]: Выход из каталога `/home/vladimir/mono/nacl/runtime-build/mono/mini' make[2]: *** [all-recursive] Ошибка 1 make[2]: Выход из каталога `/home/vladimir/mono/nacl/runtime-build/mono' make[1]: *** [all-recursive] Ошибка 1 make[1]: Выход из каталога
Re: [Mono-dev] [PATCH] more support for Google Native Client
Hi, Take a look at this: https://github.com/elijahtaylor/mono/blob/master/mono/mini/genmdesc.c This includes a dummy implementation of __nacl_suspend_thread_if_needed for that file. It wasn't merged into mono's head because it's a temporary measure and won't be required long term. Essentially for building Mono for NaCl you can try one of 2 things: 1) Get Mono 2.10 and patch in my nacl/ folder (which it looks like you did) and my change to genmdesc.c above. Downside: I haven't explicitly tested NaCl support with the very latest release, it's possible something has broken slightly since my patch landed. I'm working on a continuous build to minimize possible breakage in the future. 2) Get elijahtaylor/mono from github. Downside: I haven't merged all of the 2.10 changes in, so there may be some stuff you don't get, but you should get all of the Native Client work and it should function. And to provide a little more info, I saw your posts to the other mono-devel thread (sorry, I missed the one a few days ago). Please note that 3D support is currently in progress in Native Client and may not be suitable for public use. Also, Mono for NaCl doesn't have any bindings to any of the PPAPI interfaces that are exposed for interacting with the browser yet, so if you need such interaction, you'll have to provide it in C/C++ in the meantime; this is really a barebones implementation of Mono currently. And lastly, I haven't tested building Mono for NaCl under cygwin, but if you have the Native Client toolchain installed in cygwin, I would *assume* it should work, but you're really in uncharted waters. I've been building on Ubuntu 10.04 LTS and Mac OS X 10.5.8 with success. -Elijah On Fri, Feb 18, 2011 at 9:20 AM, C.Rivlaldo vladi...@neoaxisgroup.comwrote: Hello! I'm trying to compile Mono 2.10 for Nacl at Ubuntu with using Elijah's patch. After executing nacl-runtime-mono.sh I had some error about undefined reference: genmdesc.c:29: undefined reference to `__nacl_suspend_thread_if_needed' Why it happened? Please, help me. I want to compile mono very much. P.S. End of log which I get: /bin/bash ../../libtool --tag=CC --mode=link /home/vladimir/nacl/native_client/toolchain/linux_x86/bin/nacl-gcc -I../../../.. -I../../../../eglib/src -I../../eglib/src-g -D_POSIX_PATH_MAX=256 -DPATH_MAX=256 -fno-strict-aliasing -Wdeclaration-after-statement -g -Wall -Wunused -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Wmissing-prototypes -Wnested-externs -Wpointer-arith -Wno-cast-qual -Wwrite-strings -mno-tls-direct-seg-refs -o genmdesc genmdesc-genmdesc.o genmdesc-helpers.o genmdesc-opcodes.o ../../mono/utils/libmonoutils.la -lm -L../../eglib/src -leglib -lm -lm -lnosys -lg -lpthread libtool: link: /home/vladimir/nacl/native_client/toolchain/linux_x86/bin/nacl-gcc -I../../../.. -I../../../../eglib/src -I../../eglib/src -g -D_POSIX_PATH_MAX=256 -DPATH_MAX=256 -fno-strict-aliasing -Wdeclaration-after-statement -g -Wall -Wunused -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Wmissing-prototypes -Wnested-externs -Wpointer-arith -Wno-cast-qual -Wwrite-strings -mno-tls-direct-seg-refs -o genmdesc genmdesc-genmdesc.o genmdesc-helpers.o genmdesc-opcodes.o ../../mono/utils/.libs/libmonoutils.a -L/home/vladimir/mono/nacl/runtime-build/eglib/src /home/vladimir/mono/nacl/runtime-build/eglib/src/.libs/libeglib.a -lm -lnosys -lg -lpthread /home/vladimir/mono/nacl/runtime-build/eglib/src/.libs/libeglib.a(libeglib_la-gpath.o): In function `monoeg_g_find_program_in_path': /home/vladimir/mono/nacl/runtime-build/eglib/src/../../../../eglib/src/gpath.c:226: warning: the `access' function is not implemented and will always fail /home/vladimir/nacl/native_client/toolchain/linux_x86/bin/../lib/gcc/nacl64/4.4.3/../../../../nacl64/lib/../lib32/libg.a(lib_a-execve.o): In function `execve': execve.c:(.text+0xa): warning: the `_execve' function is not implemented and will always fail /home/vladimir/nacl/native_client/toolchain/linux_x86/bin/../lib/gcc/nacl64/4.4.3/../../../../nacl64/lib/../lib32/libg.a(lib_a-execr.o): In function `_fork_r': execr.c:(.text+0x9c): warning: the `fork' function is not implemented and will always fail /home/vladimir/mono/nacl/runtime-build/eglib/src/.libs/libeglib.a(libeglib_la-gfile-posix.o): In function `monoeg_g_get_current_dir': /home/vladimir/mono/nacl/runtime-build/eglib/src/../../../../eglib/src/gfile-posix.c:158: warning: the `getcwd' function is not implemented and will always fail /home/vladimir/mono/nacl/runtime-build/eglib/src/.libs/libeglib.a(libeglib_la-gmisc-unix.o): In function `get_pw_data': /home/vladimir/mono/nacl/runtime-build/eglib/src/../../../../eglib/src/gmisc-unix.c:97: warning: the `getpwuid_r' function is not implemented and will always fail /home/vladimir/mono/nacl/runtime-build/eglib/src/../../../../eglib/src/gmisc-unix.c:97: warning: the `getuid' function is not implemented and
Re: [Mono-dev] [PATCH] more support for Google Native Client
Hi,
I merged your changes to mono's master except for the following:
runtime/mono-wrapper.in
mono/mini/genmdesc.c
nacl/
Zoltan
On Thu, Jan 6, 2011 at 2:22 AM, Elijah Taylor elijahtay...@google.comwrote:
Ok, I'll check out the changes/info you mentioned and go through the files
that auto-merged, too. Probably won't get this done for at least a day or
so, but I'll rebase again once I've fixed it. Hopefully by that point
something else won't have broken too :)
-Elijah
On Wed, Jan 5, 2011 at 5:19 PM, Zoltan Varga var...@gmail.com wrote:
Hi,
This should work as follows: every aot image contains a MonoAotFileInfo
structure,
emitted in emit_file_info () in aot-compiler.c, which has a 'flags'
field, and the MONO_AOT_FILE_FLAG_FULL_AOT flag should be set in
this field. At runtime, check_usable() in aot-runtime.c checks this flag.
Zoltan
On Thu, Jan 6, 2011 at 2:10 AM, Zoltan Varga var...@gmail.com wrote:
Hi,
On Thu, Jan 6, 2011 at 1:24 AM, Elijah Taylor
elijahtay...@google.comwrote:
Zoltan,
I've rebased from mono's master branch and fixed all merge conflicts,
but something that's gone in since I first forked has now broken NaCl AOT
compilation for me. On amd64 the compiler just crashes and I'm looking
into
that, nut on x86 I'm getting this: Can't use AOT image 'mscorlib' in
aot-only mode because it is not compiled with --aot=full. But I'm
compiling with --aot=full,static,nodebug,ntrampolines=4096
If need be I can pick through the AOT changes that have gone in, but I
was hoping you or someone on this list would be able to tell me the major
changes to AOT from the past 3 weeks and some ideas about what might be
getting in my way. Can you shed any light?
There was a big reorganization in the AOT file format to reduce the
number of global symbols exported from the aot images. No idea why this is
causing problems. make fullaotcheck and make fsacheck still seems to work
for me on x86. I fixed a uninitilized memory error in 88d676ffd425def3,
maybe that will help.
Zoltan
-Elijah
On Wed, Jan 5, 2011 at 3:51 PM, Zoltan Varga var...@gmail.com wrote:
Hi,
I think the current code looks ok, and we should think about how to
merge it into mono trunk. As a first step, could you rebase your master
branch on top of master to fix the few conflicts which has surfaced due to
changes to mono master ?
Zoltan
On Wed, Jan 5, 2011 at 8:23 PM, Elijah Taylor elijahtay...@google.com
wrote:
Hi Zoltan,
I've addressed all of the issues you pointed out (minus genmdesc.c:
__nacl_suspend_thread_if_needed, but that doesn't need to be merged in at
this time, it can remain in my local repository only). Please take
another
look at your earliest convenience and let me know if there's anything
else
you need from me.
-Elijah
On Tue, Jan 4, 2011 at 10:55 AM, Elijah Taylor
elijahtay...@google.com wrote:
Replies inline:
On Tue, Jan 4, 2011 at 10:30 AM, Zoltan Varga var...@gmail.comwrote:
Hi,
Some comments:
- the patch changes IMT_REG to AMD64_R11 in the non-nacl case, I'm
not sure thats
intentional.
Has this changed in the last six months on the Mono side? IIRC I
didn't mean to change anything like this. The reason I made explicit
defines was so code in aot-compiler and mini-amd64 could share defines
over
which reg was the one we jump through and which was a scratch reg. I'll
diff vs Mono head revision and make it correct.
- you could define __mono_ilp32__ in the nacl/amd64 case, and use
that instead of
defined(__native_client_codegen__) defined(TARGET_AMD64) in a
few places.
That sounds reasonable. I'm assuming you mean non-arch specific
areas like mini.c, aot-*.c, method-to-ir.c, etc? Are there any other
major
consequences to defining __mono_ilp32__ ?
- it would be better to define nacl_global_codeman_validate () as a
no-op in the non-nacl case, so its callers wouldn't need #ifdefs.
I'll fix this.
- genmdesc.c contains this change, which is probably not needed:
+void __nacl_suspend_thread_if_needed() {}
+
It is needed temporarily due to a preliminary GC implementation, we
don't have to submit it this way. Eventually (soon) we won't need it at
all.
- you could use sizeof(mgreg_t) instead of SIZEOF_REGISTER to be
consistent with
the usage of sizeof(gpointer).
Sounds good. I'll try to use sizeof for all compiled code and only
use SIZEOF_REGISTER/SIZEOF_VOID_P for pre-processor directives only.
Other than these, I think the changes look fine, they aren't that
disruptive, since they don't
change the non-nacl behavior at all.
Great! I was worried just based on LOC changed that it might get
more resistance. In truth I'm more worried about future Mono changes
accidentally breaking NaCl behavior. I'm planning on getting some
automated
testing implemented soon to
Re: [Mono-dev] [PATCH] more support for Google Native Client
Hi,
I had to revert this change, as it was causing crashes on amd64:
@@ -357,8 +494,10 @@ mono_code_manager_reserve_align (MonoCodeManager *cman,
int size, int alignment)
for (chunk = cman-current; chunk; chunk = chunk-next) {
if (ALIGN_INT (chunk-pos, alignment) + size = chunk-size) {
chunk-pos = ALIGN_INT (chunk-pos, alignment);
- ptr = chunk-data + chunk-pos;
- chunk-pos += size;
+ /* Align the chunk-data we add to chunk-pos */
+ /* or we can't guarantee proper alignment */
+ ptr = (void*)uintptr_t)chunk-data + align_mask) ~align_mask) +
chunk-pos);
+ chunk-pos = ((char*)ptr - chunk-data) + size;
return ptr;
}
}
it was inside a #ifndef native_client, so why is this needed ?
Zoltan
On Thu, Jan 6, 2011 at 11:34 AM, Zoltan Varga var...@gmail.com wrote:
Hi,
I merged your changes to mono's master except for the following:
runtime/mono-wrapper.in
mono/mini/genmdesc.c
nacl/
Zoltan
On Thu, Jan 6, 2011 at 2:22 AM, Elijah Taylor elijahtay...@google.comwrote:
Ok, I'll check out the changes/info you mentioned and go through the files
that auto-merged, too. Probably won't get this done for at least a day or
so, but I'll rebase again once I've fixed it. Hopefully by that point
something else won't have broken too :)
-Elijah
On Wed, Jan 5, 2011 at 5:19 PM, Zoltan Varga var...@gmail.com wrote:
Hi,
This should work as follows: every aot image contains a MonoAotFileInfo
structure,
emitted in emit_file_info () in aot-compiler.c, which has a 'flags'
field, and the MONO_AOT_FILE_FLAG_FULL_AOT flag should be set in
this field. At runtime, check_usable() in aot-runtime.c checks this flag.
Zoltan
On Thu, Jan 6, 2011 at 2:10 AM, Zoltan Varga var...@gmail.com wrote:
Hi,
On Thu, Jan 6, 2011 at 1:24 AM, Elijah Taylor
elijahtay...@google.comwrote:
Zoltan,
I've rebased from mono's master branch and fixed all merge conflicts,
but something that's gone in since I first forked has now broken NaCl AOT
compilation for me. On amd64 the compiler just crashes and I'm looking
into
that, nut on x86 I'm getting this: Can't use AOT image 'mscorlib' in
aot-only mode because it is not compiled with --aot=full. But I'm
compiling with --aot=full,static,nodebug,ntrampolines=4096
If need be I can pick through the AOT changes that have gone in, but I
was hoping you or someone on this list would be able to tell me the major
changes to AOT from the past 3 weeks and some ideas about what might be
getting in my way. Can you shed any light?
There was a big reorganization in the AOT file format to reduce the
number of global symbols exported from the aot images. No idea why this is
causing problems. make fullaotcheck and make fsacheck still seems to work
for me on x86. I fixed a uninitilized memory error in 88d676ffd425def3,
maybe that will help.
Zoltan
-Elijah
On Wed, Jan 5, 2011 at 3:51 PM, Zoltan Varga var...@gmail.com wrote:
Hi,
I think the current code looks ok, and we should think about how to
merge it into mono trunk. As a first step, could you rebase your master
branch on top of master to fix the few conflicts which has surfaced due
to
changes to mono master ?
Zoltan
On Wed, Jan 5, 2011 at 8:23 PM, Elijah Taylor
elijahtay...@google.com wrote:
Hi Zoltan,
I've addressed all of the issues you pointed out (minus genmdesc.c:
__nacl_suspend_thread_if_needed, but that doesn't need to be merged in
at
this time, it can remain in my local repository only). Please take
another
look at your earliest convenience and let me know if there's anything
else
you need from me.
-Elijah
On Tue, Jan 4, 2011 at 10:55 AM, Elijah Taylor
elijahtay...@google.com wrote:
Replies inline:
On Tue, Jan 4, 2011 at 10:30 AM, Zoltan Varga var...@gmail.comwrote:
Hi,
Some comments:
- the patch changes IMT_REG to AMD64_R11 in the non-nacl case, I'm
not sure thats
intentional.
Has this changed in the last six months on the Mono side? IIRC I
didn't mean to change anything like this. The reason I made explicit
defines was so code in aot-compiler and mini-amd64 could share defines
over
which reg was the one we jump through and which was a scratch reg.
I'll
diff vs Mono head revision and make it correct.
- you could define __mono_ilp32__ in the nacl/amd64 case, and use
that instead of
defined(__native_client_codegen__) defined(TARGET_AMD64) in a
few places.
That sounds reasonable. I'm assuming you mean non-arch specific
areas like mini.c, aot-*.c, method-to-ir.c, etc? Are there any other
major
consequences to defining __mono_ilp32__ ?
- it would be better to define nacl_global_codeman_validate () as a
no-op in the non-nacl case, so its callers wouldn't need #ifdefs.
I'll fix this.
- genmdesc.c contains this change, which is probably not needed:
+void
Re: [Mono-dev] [PATCH] more support for Google Native Client
This bit of code runs for our AOT compiler, but not for our JIT (in that
case, it's a native 32-bit app that defines __native_client_codegen__).
The problem is that chunk-data isn't guaranteed to be aligned to MIN_ALIGN
**or** the alignment you pass into *mono_code_manager_reserve_align* if you
use mono_valloc instead of dlmemalign in *new_codechunk*. But chunk-pos is
aligned to the alignment passed in, so the returned pointer could be
misaligned.
As you can see in the alloc function I adjusted it to give MIN_ALIGN - 1
extra bytes to account for this slop, and there's an identical piece to this
below for new codechunks that are allocated. I'm curious why this is
causing problems for non-nacl builds... if chunk-data is aligned this
should essentially be a no-op, and if it's not aligned, this code is
supposed to fix that. Is there a simple test I can run to see the failure?
On Thu, Jan 6, 2011 at 3:43 AM, Zoltan Varga var...@gmail.com wrote:
Hi,
I had to revert this change, as it was causing crashes on amd64:
@@ -357,8 +494,10 @@ mono_code_manager_reserve_align (MonoCodeManager
*cman, int size, int alignment)
for (chunk = cman-current; chunk; chunk = chunk-next) {
if (ALIGN_INT (chunk-pos, alignment) + size = chunk-size) {
chunk-pos = ALIGN_INT (chunk-pos, alignment);
- ptr = chunk-data + chunk-pos;
- chunk-pos += size;
+ /* Align the chunk-data we add to chunk-pos */
+ /* or we can't guarantee proper alignment */
+ ptr = (void*)uintptr_t)chunk-data + align_mask) ~align_mask) +
chunk-pos);
+ chunk-pos = ((char*)ptr - chunk-data) + size;
return ptr;
}
}
it was inside a #ifndef native_client, so why is this needed ?
Zoltan
On Thu, Jan 6, 2011 at 11:34 AM, Zoltan Varga var...@gmail.com wrote:
Hi,
I merged your changes to mono's master except for the following:
runtime/mono-wrapper.in
mono/mini/genmdesc.c
nacl/
Zoltan
On Thu, Jan 6, 2011 at 2:22 AM, Elijah Taylor elijahtay...@google.comwrote:
Ok, I'll check out the changes/info you mentioned and go through the
files that auto-merged, too. Probably won't get this done for at least a
day or so, but I'll rebase again once I've fixed it. Hopefully by that
point something else won't have broken too :)
-Elijah
On Wed, Jan 5, 2011 at 5:19 PM, Zoltan Varga var...@gmail.com wrote:
Hi,
This should work as follows: every aot image contains a
MonoAotFileInfo structure,
emitted in emit_file_info () in aot-compiler.c, which has a 'flags'
field, and the MONO_AOT_FILE_FLAG_FULL_AOT flag should be set in
this field. At runtime, check_usable() in aot-runtime.c checks this
flag.
Zoltan
On Thu, Jan 6, 2011 at 2:10 AM, Zoltan Varga var...@gmail.com wrote:
Hi,
On Thu, Jan 6, 2011 at 1:24 AM, Elijah Taylor elijahtay...@google.com
wrote:
Zoltan,
I've rebased from mono's master branch and fixed all merge conflicts,
but something that's gone in since I first forked has now broken NaCl AOT
compilation for me. On amd64 the compiler just crashes and I'm looking
into
that, nut on x86 I'm getting this: Can't use AOT image 'mscorlib' in
aot-only mode because it is not compiled with --aot=full. But I'm
compiling with --aot=full,static,nodebug,ntrampolines=4096
If need be I can pick through the AOT changes that have gone in, but I
was hoping you or someone on this list would be able to tell me the major
changes to AOT from the past 3 weeks and some ideas about what might be
getting in my way. Can you shed any light?
There was a big reorganization in the AOT file format to reduce the
number of global symbols exported from the aot images. No idea why this is
causing problems. make fullaotcheck and make fsacheck still seems to work
for me on x86. I fixed a uninitilized memory error in 88d676ffd425def3,
maybe that will help.
Zoltan
-Elijah
On Wed, Jan 5, 2011 at 3:51 PM, Zoltan Varga var...@gmail.comwrote:
Hi,
I think the current code looks ok, and we should think about how to
merge it into mono trunk. As a first step, could you rebase your master
branch on top of master to fix the few conflicts which has surfaced due
to
changes to mono master ?
Zoltan
On Wed, Jan 5, 2011 at 8:23 PM, Elijah Taylor
elijahtay...@google.com wrote:
Hi Zoltan,
I've addressed all of the issues you pointed out (minus genmdesc.c:
__nacl_suspend_thread_if_needed, but that doesn't need to be merged in
at
this time, it can remain in my local repository only). Please take
another
look at your earliest convenience and let me know if there's anything
else
you need from me.
-Elijah
On Tue, Jan 4, 2011 at 10:55 AM, Elijah Taylor
elijahtay...@google.com wrote:
Replies inline:
On Tue, Jan 4, 2011 at 10:30 AM, Zoltan Varga var...@gmail.comwrote:
Hi,
Some comments:
- the patch changes IMT_REG to AMD64_R11 in the non-nacl
Re: [Mono-dev] [PATCH] more support for Google Native Client
Hi,
Its fixed now, it was missing a (uintptr_t) cast around align_mask.
mono_valloc () is supposed to return pagesize aligned memory, isn't that
enough ?
Zoltan
On Thu, Jan 6, 2011 at 8:14 PM, Elijah Taylor elijahtay...@google.comwrote:
This bit of code runs for our AOT compiler, but not for our JIT (in that
case, it's a native 32-bit app that defines __native_client_codegen__).
The problem is that chunk-data isn't guaranteed to be aligned to MIN_ALIGN
**or** the alignment you pass into *mono_code_manager_reserve_align* if
you use mono_valloc instead of dlmemalign in *new_codechunk*. But
chunk-pos is aligned to the alignment passed in, so the returned pointer
could be misaligned.
As you can see in the alloc function I adjusted it to give MIN_ALIGN - 1
extra bytes to account for this slop, and there's an identical piece to this
below for new codechunks that are allocated. I'm curious why this is
causing problems for non-nacl builds... if chunk-data is aligned this
should essentially be a no-op, and if it's not aligned, this code is
supposed to fix that. Is there a simple test I can run to see the failure?
On Thu, Jan 6, 2011 at 3:43 AM, Zoltan Varga var...@gmail.com wrote:
Hi,
I had to revert this change, as it was causing crashes on amd64:
@@ -357,8 +494,10 @@ mono_code_manager_reserve_align (MonoCodeManager
*cman, int size, int alignment)
for (chunk = cman-current; chunk; chunk = chunk-next) {
if (ALIGN_INT (chunk-pos, alignment) + size = chunk-size) {
chunk-pos = ALIGN_INT (chunk-pos, alignment);
- ptr = chunk-data + chunk-pos;
- chunk-pos += size;
+ /* Align the chunk-data we add to chunk-pos */
+ /* or we can't guarantee proper alignment */
+ ptr = (void*)uintptr_t)chunk-data + align_mask) ~align_mask) +
chunk-pos);
+ chunk-pos = ((char*)ptr - chunk-data) + size;
return ptr;
}
}
it was inside a #ifndef native_client, so why is this needed ?
Zoltan
On Thu, Jan 6, 2011 at 11:34 AM, Zoltan Varga var...@gmail.com wrote:
Hi,
I merged your changes to mono's master except for the following:
runtime/mono-wrapper.in
mono/mini/genmdesc.c
nacl/
Zoltan
On Thu, Jan 6, 2011 at 2:22 AM, Elijah Taylor
elijahtay...@google.comwrote:
Ok, I'll check out the changes/info you mentioned and go through the
files that auto-merged, too. Probably won't get this done for at least a
day or so, but I'll rebase again once I've fixed it. Hopefully by that
point something else won't have broken too :)
-Elijah
On Wed, Jan 5, 2011 at 5:19 PM, Zoltan Varga var...@gmail.com wrote:
Hi,
This should work as follows: every aot image contains a
MonoAotFileInfo structure,
emitted in emit_file_info () in aot-compiler.c, which has a 'flags'
field, and the MONO_AOT_FILE_FLAG_FULL_AOT flag should be set in
this field. At runtime, check_usable() in aot-runtime.c checks this
flag.
Zoltan
On Thu, Jan 6, 2011 at 2:10 AM, Zoltan Varga var...@gmail.com wrote:
Hi,
On Thu, Jan 6, 2011 at 1:24 AM, Elijah Taylor
elijahtay...@google.com wrote:
Zoltan,
I've rebased from mono's master branch and fixed all merge conflicts,
but something that's gone in since I first forked has now broken NaCl
AOT
compilation for me. On amd64 the compiler just crashes and I'm looking
into
that, nut on x86 I'm getting this: Can't use AOT image 'mscorlib' in
aot-only mode because it is not compiled with --aot=full. But I'm
compiling with --aot=full,static,nodebug,ntrampolines=4096
If need be I can pick through the AOT changes that have gone in, but
I was hoping you or someone on this list would be able to tell me the
major
changes to AOT from the past 3 weeks and some ideas about what might be
getting in my way. Can you shed any light?
There was a big reorganization in the AOT file format to reduce the
number of global symbols exported from the aot images. No idea why this
is
causing problems. make fullaotcheck and make fsacheck still seems to work
for me on x86. I fixed a uninitilized memory error in 88d676ffd425def3,
maybe that will help.
Zoltan
-Elijah
On Wed, Jan 5, 2011 at 3:51 PM, Zoltan Varga var...@gmail.comwrote:
Hi,
I think the current code looks ok, and we should think about how
to merge it into mono trunk. As a first step, could you rebase your
master
branch on top of master to fix the few conflicts which has surfaced
due to
changes to mono master ?
Zoltan
On Wed, Jan 5, 2011 at 8:23 PM, Elijah Taylor
elijahtay...@google.com wrote:
Hi Zoltan,
I've addressed all of the issues you pointed out (minus genmdesc.c:
__nacl_suspend_thread_if_needed, but that doesn't need to be merged
in at
this time, it can remain in my local repository only). Please take
another
look at your earliest convenience and let me know if there's anything
else
Re: [Mono-dev] [PATCH] more support for Google Native Client
Page alignment would be good enough, but I wasn't clear because I had
forgotten some details. When I saw this behavior we were using the dummy
implementation of mono_valloc which falls back on malloc, which I don't
believe we use anymore for the AOT compiler since it's a native app.
This code is probably not necessary for NaCl anymore, but if anyone is using
this dummy implementation then it's still a valid fix.
On Thu, Jan 6, 2011 at 11:31 AM, Zoltan Varga var...@gmail.com wrote:
Hi,
Its fixed now, it was missing a (uintptr_t) cast around align_mask.
mono_valloc () is supposed to return pagesize aligned memory, isn't that
enough ?
Zoltan
On Thu, Jan 6, 2011 at 8:14 PM, Elijah Taylor elijahtay...@google.comwrote:
This bit of code runs for our AOT compiler, but not for our JIT (in that
case, it's a native 32-bit app that defines __native_client_codegen__).
The problem is that chunk-data isn't guaranteed to be aligned to
MIN_ALIGN **or** the alignment you pass into *
mono_code_manager_reserve_align* if you use mono_valloc instead of
dlmemalign in *new_codechunk*. But chunk-pos is aligned to the
alignment passed in, so the returned pointer could be misaligned.
As you can see in the alloc function I adjusted it to give MIN_ALIGN - 1
extra bytes to account for this slop, and there's an identical piece to this
below for new codechunks that are allocated. I'm curious why this is
causing problems for non-nacl builds... if chunk-data is aligned this
should essentially be a no-op, and if it's not aligned, this code is
supposed to fix that. Is there a simple test I can run to see the failure?
On Thu, Jan 6, 2011 at 3:43 AM, Zoltan Varga var...@gmail.com wrote:
Hi,
I had to revert this change, as it was causing crashes on amd64:
@@ -357,8 +494,10 @@ mono_code_manager_reserve_align (MonoCodeManager
*cman, int size, int alignment)
for (chunk = cman-current; chunk; chunk = chunk-next) {
if (ALIGN_INT (chunk-pos, alignment) + size = chunk-size) {
chunk-pos = ALIGN_INT (chunk-pos, alignment);
- ptr = chunk-data + chunk-pos;
- chunk-pos += size;
+ /* Align the chunk-data we add to chunk-pos */
+ /* or we can't guarantee proper alignment */
+ ptr = (void*)uintptr_t)chunk-data + align_mask) ~align_mask) +
chunk-pos);
+ chunk-pos = ((char*)ptr - chunk-data) + size;
return ptr;
}
}
it was inside a #ifndef native_client, so why is this needed ?
Zoltan
On Thu, Jan 6, 2011 at 11:34 AM, Zoltan Varga var...@gmail.com wrote:
Hi,
I merged your changes to mono's master except for the following:
runtime/mono-wrapper.in
mono/mini/genmdesc.c
nacl/
Zoltan
On Thu, Jan 6, 2011 at 2:22 AM, Elijah Taylor
elijahtay...@google.comwrote:
Ok, I'll check out the changes/info you mentioned and go through the
files that auto-merged, too. Probably won't get this done for at least a
day or so, but I'll rebase again once I've fixed it. Hopefully by that
point something else won't have broken too :)
-Elijah
On Wed, Jan 5, 2011 at 5:19 PM, Zoltan Varga var...@gmail.com wrote:
Hi,
This should work as follows: every aot image contains a
MonoAotFileInfo structure,
emitted in emit_file_info () in aot-compiler.c, which has a 'flags'
field, and the MONO_AOT_FILE_FLAG_FULL_AOT flag should be set in
this field. At runtime, check_usable() in aot-runtime.c checks this
flag.
Zoltan
On Thu, Jan 6, 2011 at 2:10 AM, Zoltan Varga var...@gmail.comwrote:
Hi,
On Thu, Jan 6, 2011 at 1:24 AM, Elijah Taylor
elijahtay...@google.com wrote:
Zoltan,
I've rebased from mono's master branch and fixed all merge
conflicts, but something that's gone in since I first forked has now
broken
NaCl AOT compilation for me. On amd64 the compiler just crashes and
I'm
looking into that, nut on x86 I'm getting this: Can't use AOT image
'mscorlib' in aot-only mode because it is not compiled with
--aot=full. But
I'm compiling with --aot=full,static,nodebug,ntrampolines=4096
If need be I can pick through the AOT changes that have gone in, but
I was hoping you or someone on this list would be able to tell me the
major
changes to AOT from the past 3 weeks and some ideas about what might be
getting in my way. Can you shed any light?
There was a big reorganization in the AOT file format to reduce the
number of global symbols exported from the aot images. No idea why this
is
causing problems. make fullaotcheck and make fsacheck still seems to
work
for me on x86. I fixed a uninitilized memory error in 88d676ffd425def3,
maybe that will help.
Zoltan
-Elijah
On Wed, Jan 5, 2011 at 3:51 PM, Zoltan Varga var...@gmail.comwrote:
Hi,
I think the current code looks ok, and we should think about how
to merge it into mono trunk. As a first step, could you rebase your
master
branch on top of master
Re: [Mono-dev] [PATCH] more support for Google Native Client
Also, if any of the memory allocation routines don't get a larger alignment
like pages, and instead just use MIN_ALIGN, yet pass in a larger alignment
requirement to *mono_code_manager_reserve_align*, this code would be needed
too. (sounds a bit contrived I guess :)
On Thu, Jan 6, 2011 at 11:41 AM, Elijah Taylor elijahtay...@google.comwrote:
Page alignment would be good enough, but I wasn't clear because I had
forgotten some details. When I saw this behavior we were using the dummy
implementation of mono_valloc which falls back on malloc, which I don't
believe we use anymore for the AOT compiler since it's a native app.
This code is probably not necessary for NaCl anymore, but if anyone is
using this dummy implementation then it's still a valid fix.
On Thu, Jan 6, 2011 at 11:31 AM, Zoltan Varga var...@gmail.com wrote:
Hi,
Its fixed now, it was missing a (uintptr_t) cast around align_mask.
mono_valloc () is supposed to return pagesize aligned memory, isn't that
enough ?
Zoltan
On Thu, Jan 6, 2011 at 8:14 PM, Elijah Taylor elijahtay...@google.comwrote:
This bit of code runs for our AOT compiler, but not for our JIT (in that
case, it's a native 32-bit app that defines __native_client_codegen__).
The problem is that chunk-data isn't guaranteed to be aligned to
MIN_ALIGN **or** the alignment you pass into *
mono_code_manager_reserve_align* if you use mono_valloc instead of
dlmemalign in *new_codechunk*. But chunk-pos is aligned to the
alignment passed in, so the returned pointer could be misaligned.
As you can see in the alloc function I adjusted it to give MIN_ALIGN - 1
extra bytes to account for this slop, and there's an identical piece to this
below for new codechunks that are allocated. I'm curious why this is
causing problems for non-nacl builds... if chunk-data is aligned this
should essentially be a no-op, and if it's not aligned, this code is
supposed to fix that. Is there a simple test I can run to see the failure?
On Thu, Jan 6, 2011 at 3:43 AM, Zoltan Varga var...@gmail.com wrote:
Hi,
I had to revert this change, as it was causing crashes on amd64:
@@ -357,8 +494,10 @@ mono_code_manager_reserve_align (MonoCodeManager
*cman, int size, int alignment)
for (chunk = cman-current; chunk; chunk = chunk-next) {
if (ALIGN_INT (chunk-pos, alignment) + size = chunk-size) {
chunk-pos = ALIGN_INT (chunk-pos, alignment);
- ptr = chunk-data + chunk-pos;
- chunk-pos += size;
+ /* Align the chunk-data we add to chunk-pos */
+ /* or we can't guarantee proper alignment */
+ ptr = (void*)uintptr_t)chunk-data + align_mask) ~align_mask) +
chunk-pos);
+ chunk-pos = ((char*)ptr - chunk-data) + size;
return ptr;
}
}
it was inside a #ifndef native_client, so why is this needed ?
Zoltan
On Thu, Jan 6, 2011 at 11:34 AM, Zoltan Varga var...@gmail.com wrote:
Hi,
I merged your changes to mono's master except for the following:
runtime/mono-wrapper.in
mono/mini/genmdesc.c
nacl/
Zoltan
On Thu, Jan 6, 2011 at 2:22 AM, Elijah Taylor elijahtay...@google.com
wrote:
Ok, I'll check out the changes/info you mentioned and go through the
files that auto-merged, too. Probably won't get this done for at least a
day or so, but I'll rebase again once I've fixed it. Hopefully by that
point something else won't have broken too :)
-Elijah
On Wed, Jan 5, 2011 at 5:19 PM, Zoltan Varga var...@gmail.comwrote:
Hi,
This should work as follows: every aot image contains a
MonoAotFileInfo structure,
emitted in emit_file_info () in aot-compiler.c, which has a 'flags'
field, and the MONO_AOT_FILE_FLAG_FULL_AOT flag should be set in
this field. At runtime, check_usable() in aot-runtime.c checks this
flag.
Zoltan
On Thu, Jan 6, 2011 at 2:10 AM, Zoltan Varga var...@gmail.comwrote:
Hi,
On Thu, Jan 6, 2011 at 1:24 AM, Elijah Taylor
elijahtay...@google.com wrote:
Zoltan,
I've rebased from mono's master branch and fixed all merge
conflicts, but something that's gone in since I first forked has now
broken
NaCl AOT compilation for me. On amd64 the compiler just crashes and
I'm
looking into that, nut on x86 I'm getting this: Can't use AOT
image 'mscorlib' in aot-only mode because it is not compiled with
--aot=full. But I'm compiling with
--aot=full,static,nodebug,ntrampolines=4096
If need be I can pick through the AOT changes that have gone in,
but I was hoping you or someone on this list would be able to tell me
the
major changes to AOT from the past 3 weeks and some ideas about what
might
be getting in my way. Can you shed any light?
There was a big reorganization in the AOT file format to reduce the
number of global symbols exported from the aot images. No idea why
this is
causing problems. make fullaotcheck and make fsacheck still seems to
work
for me on x86. I fixed a uninitilized memory
Re: [Mono-dev] [PATCH] more support for Google Native Client
Hi Zoltan,
I've rebased my fork after your merge and subsequent fixes, and everything
seems to be working great with NaCl (I think the amd64 crash you were seeing
might have been the same one I was seeing). Thanks for the help with
reviewing the code and merging it in, I'm really glad we finally have these
changes upstream.
-Elijah
On Thu, Jan 6, 2011 at 11:50 AM, Elijah Taylor elijahtay...@google.comwrote:
Also, if any of the memory allocation routines don't get a larger alignment
like pages, and instead just use MIN_ALIGN, yet pass in a larger alignment
requirement to *mono_code_manager_reserve_align*, this code would be
needed too. (sounds a bit contrived I guess :)
On Thu, Jan 6, 2011 at 11:41 AM, Elijah Taylor elijahtay...@google.comwrote:
Page alignment would be good enough, but I wasn't clear because I had
forgotten some details. When I saw this behavior we were using the dummy
implementation of mono_valloc which falls back on malloc, which I don't
believe we use anymore for the AOT compiler since it's a native app.
This code is probably not necessary for NaCl anymore, but if anyone is
using this dummy implementation then it's still a valid fix.
On Thu, Jan 6, 2011 at 11:31 AM, Zoltan Varga var...@gmail.com wrote:
Hi,
Its fixed now, it was missing a (uintptr_t) cast around align_mask.
mono_valloc () is supposed to return pagesize aligned memory, isn't that
enough ?
Zoltan
On Thu, Jan 6, 2011 at 8:14 PM, Elijah Taylor
elijahtay...@google.comwrote:
This bit of code runs for our AOT compiler, but not for our JIT (in that
case, it's a native 32-bit app that defines __native_client_codegen__).
The problem is that chunk-data isn't guaranteed to be aligned to
MIN_ALIGN **or** the alignment you pass into *
mono_code_manager_reserve_align* if you use mono_valloc instead of
dlmemalign in *new_codechunk*. But chunk-pos is aligned to the
alignment passed in, so the returned pointer could be misaligned.
As you can see in the alloc function I adjusted it to give MIN_ALIGN - 1
extra bytes to account for this slop, and there's an identical piece to
this
below for new codechunks that are allocated. I'm curious why this is
causing problems for non-nacl builds... if chunk-data is aligned this
should essentially be a no-op, and if it's not aligned, this code is
supposed to fix that. Is there a simple test I can run to see the failure?
On Thu, Jan 6, 2011 at 3:43 AM, Zoltan Varga var...@gmail.com wrote:
Hi,
I had to revert this change, as it was causing crashes on amd64:
@@ -357,8 +494,10 @@ mono_code_manager_reserve_align (MonoCodeManager
*cman, int size, int alignment)
for (chunk = cman-current; chunk; chunk = chunk-next) {
if (ALIGN_INT (chunk-pos, alignment) + size = chunk-size) {
chunk-pos = ALIGN_INT (chunk-pos, alignment);
- ptr = chunk-data + chunk-pos;
- chunk-pos += size;
+ /* Align the chunk-data we add to chunk-pos */
+ /* or we can't guarantee proper alignment */
+ ptr = (void*)uintptr_t)chunk-data + align_mask) ~align_mask)
+ chunk-pos);
+ chunk-pos = ((char*)ptr - chunk-data) + size;
return ptr;
}
}
it was inside a #ifndef native_client, so why is this needed ?
Zoltan
On Thu, Jan 6, 2011 at 11:34 AM, Zoltan Varga var...@gmail.comwrote:
Hi,
I merged your changes to mono's master except for the following:
runtime/mono-wrapper.in
mono/mini/genmdesc.c
nacl/
Zoltan
On Thu, Jan 6, 2011 at 2:22 AM, Elijah Taylor
elijahtay...@google.com wrote:
Ok, I'll check out the changes/info you mentioned and go through the
files that auto-merged, too. Probably won't get this done for at least
a
day or so, but I'll rebase again once I've fixed it. Hopefully by that
point something else won't have broken too :)
-Elijah
On Wed, Jan 5, 2011 at 5:19 PM, Zoltan Varga var...@gmail.comwrote:
Hi,
This should work as follows: every aot image contains a
MonoAotFileInfo structure,
emitted in emit_file_info () in aot-compiler.c, which has a 'flags'
field, and the MONO_AOT_FILE_FLAG_FULL_AOT flag should be set in
this field. At runtime, check_usable() in aot-runtime.c checks this
flag.
Zoltan
On Thu, Jan 6, 2011 at 2:10 AM, Zoltan Varga var...@gmail.comwrote:
Hi,
On Thu, Jan 6, 2011 at 1:24 AM, Elijah Taylor
elijahtay...@google.com wrote:
Zoltan,
I've rebased from mono's master branch and fixed all merge
conflicts, but something that's gone in since I first forked has now
broken
NaCl AOT compilation for me. On amd64 the compiler just crashes and
I'm
looking into that, nut on x86 I'm getting this: Can't use AOT
image 'mscorlib' in aot-only mode because it is not compiled with
--aot=full. But I'm compiling with
--aot=full,static,nodebug,ntrampolines=4096
If need be I can pick through the AOT changes that have gone in,
but I was hoping you or someone on this list
Re: [Mono-dev] [PATCH] more support for Google Native Client
Hi Zoltan,
I've addressed all of the issues you pointed out (minus genmdesc.c:
__nacl_suspend_thread_if_needed, but that doesn't need to be merged in at
this time, it can remain in my local repository only). Please take another
look at your earliest convenience and let me know if there's anything else
you need from me.
-Elijah
On Tue, Jan 4, 2011 at 10:55 AM, Elijah Taylor elijahtay...@google.comwrote:
Replies inline:
On Tue, Jan 4, 2011 at 10:30 AM, Zoltan Varga var...@gmail.com wrote:
Hi,
Some comments:
- the patch changes IMT_REG to AMD64_R11 in the non-nacl case, I'm not
sure thats
intentional.
Has this changed in the last six months on the Mono side? IIRC I didn't
mean to change anything like this. The reason I made explicit defines was
so code in aot-compiler and mini-amd64 could share defines over which reg
was the one we jump through and which was a scratch reg. I'll diff vs Mono
head revision and make it correct.
- you could define __mono_ilp32__ in the nacl/amd64 case, and use that
instead of
defined(__native_client_codegen__) defined(TARGET_AMD64) in a few
places.
That sounds reasonable. I'm assuming you mean non-arch specific areas like
mini.c, aot-*.c, method-to-ir.c, etc? Are there any other major
consequences to defining __mono_ilp32__ ?
- it would be better to define nacl_global_codeman_validate () as a no-op
in the non-nacl case, so its callers wouldn't need #ifdefs.
I'll fix this.
- genmdesc.c contains this change, which is probably not needed:
+void __nacl_suspend_thread_if_needed() {}
+
It is needed temporarily due to a preliminary GC implementation, we don't
have to submit it this way. Eventually (soon) we won't need it at all.
- you could use sizeof(mgreg_t) instead of SIZEOF_REGISTER to be
consistent with
the usage of sizeof(gpointer).
Sounds good. I'll try to use sizeof for all compiled code and only use
SIZEOF_REGISTER/SIZEOF_VOID_P for pre-processor directives only.
Other than these, I think the changes look fine, they aren't that
disruptive, since they don't
change the non-nacl behavior at all.
Great! I was worried just based on LOC changed that it might get more
resistance. In truth I'm more worried about future Mono changes
accidentally breaking NaCl behavior. I'm planning on getting some automated
testing implemented soon to combat this though.
On Tue, Dec 21, 2010 at 9:12 PM, Elijah Taylor
elijahtay...@google.comwrote:
Greetings Mono developers!
*[tl;dr very large patch for Native
Clienthttp://www.chromium.org/nativeclient support
hosted here https://github.com/elijahtaylor/mono, would love feedback
and many eyes to look at it]
*
I'm back with another round of changes for supporting Google's Native
Client (NaCl), including support for amd64, JIT compilation, and Garbage
Collection. It's a large set of changes, forked on Dec 14 in github @
https://github.com/elijahtaylor/mono. I would appreciate feedback on
these changes... to facilitate this, I'll try to explain the largest changes
by feature (please email if clarification is needed):
*1) amd64 codegen*
- Rules located here:
http://www.chromium.org/nativeclient/design-documents/nacl-sfi-model-on-x86-64-systems
- Removed %r15 from register allocation, LMF save/restore, etc.
(r15 is special and not modifiable by untrusted code)
- Sandbox all data access through membase address mode. If not
%rsp or %rbp relative, re-write as clearing upper 32-bits + memindex
addressing
- align functions, call sites
- Sandbox returns and all indirect jumps (need to be 32-byte
aligned, cleared upper 32-bits)
- Never omit frame pointer as general operations to rbp aren't
allowed
*2) NaCl x86-64 is ILP32 (this is the largest set of changes and may
make some mono devs unhappy)*
- Set SIZEOF_REGISTER == 8 while sizeof(gpointer) == 4 for NaCl amd64
(we can use 8-byte instructions, but pointers are 4-bytes)
- Re-write large portions of mini-amd64.c, tramp-amd64.c,
exceptions-amd64.c, mini.c, method-to-ir.c to use appropriate sizes
(SIZEOF_REGISTER, sizeof(gpointer), literal '8'). *These changes are
disruptive, but ultimately they should be more correct than what was
there
before. *It's our opinion that these changes actually improve Mono
despite their impact.
- We only generate NaCl amd64 code from an ILP32 machine (either a
32-bit application for AOT code, or NaCl runtime JIT), so we may not have
caught all of the [8 -- SIZEOF_REGISTER] conversions, but we likely
caught
most of the [sizeof(gpointer) -- SIZEOF_REGISTER] and [8 --
sizeof(gpointer)] changes that are necessary.
- Change atomic operations and default pointer directives to use
32-bit instructions (long instead of quad)
- Change default operations to use 32-bit integers/pointers (eg,
OP_LOAD_MEMBASE uses 4-bytes
Re: [Mono-dev] [PATCH] more support for Google Native Client
Hi,
I think the current code looks ok, and we should think about how to merge
it into mono trunk. As a first step, could you rebase your master branch on
top of master to fix the few conflicts which has surfaced due to changes to
mono master ?
Zoltan
On Wed, Jan 5, 2011 at 8:23 PM, Elijah Taylor elijahtay...@google.comwrote:
Hi Zoltan,
I've addressed all of the issues you pointed out (minus genmdesc.c:
__nacl_suspend_thread_if_needed, but that doesn't need to be merged in at
this time, it can remain in my local repository only). Please take another
look at your earliest convenience and let me know if there's anything else
you need from me.
-Elijah
On Tue, Jan 4, 2011 at 10:55 AM, Elijah Taylor elijahtay...@google.comwrote:
Replies inline:
On Tue, Jan 4, 2011 at 10:30 AM, Zoltan Varga var...@gmail.com wrote:
Hi,
Some comments:
- the patch changes IMT_REG to AMD64_R11 in the non-nacl case, I'm not
sure thats
intentional.
Has this changed in the last six months on the Mono side? IIRC I didn't
mean to change anything like this. The reason I made explicit defines was
so code in aot-compiler and mini-amd64 could share defines over which reg
was the one we jump through and which was a scratch reg. I'll diff vs Mono
head revision and make it correct.
- you could define __mono_ilp32__ in the nacl/amd64 case, and use that
instead of
defined(__native_client_codegen__) defined(TARGET_AMD64) in a few
places.
That sounds reasonable. I'm assuming you mean non-arch specific areas
like mini.c, aot-*.c, method-to-ir.c, etc? Are there any other major
consequences to defining __mono_ilp32__ ?
- it would be better to define nacl_global_codeman_validate () as a no-op
in the non-nacl case, so its callers wouldn't need #ifdefs.
I'll fix this.
- genmdesc.c contains this change, which is probably not needed:
+void __nacl_suspend_thread_if_needed() {}
+
It is needed temporarily due to a preliminary GC implementation, we don't
have to submit it this way. Eventually (soon) we won't need it at all.
- you could use sizeof(mgreg_t) instead of SIZEOF_REGISTER to be
consistent with
the usage of sizeof(gpointer).
Sounds good. I'll try to use sizeof for all compiled code and only use
SIZEOF_REGISTER/SIZEOF_VOID_P for pre-processor directives only.
Other than these, I think the changes look fine, they aren't that
disruptive, since they don't
change the non-nacl behavior at all.
Great! I was worried just based on LOC changed that it might get more
resistance. In truth I'm more worried about future Mono changes
accidentally breaking NaCl behavior. I'm planning on getting some automated
testing implemented soon to combat this though.
On Tue, Dec 21, 2010 at 9:12 PM, Elijah Taylor
elijahtay...@google.comwrote:
Greetings Mono developers!
*[tl;dr very large patch for Native
Clienthttp://www.chromium.org/nativeclient support
hosted here https://github.com/elijahtaylor/mono, would love feedback
and many eyes to look at it]
*
I'm back with another round of changes for supporting Google's Native
Client (NaCl), including support for amd64, JIT compilation, and Garbage
Collection. It's a large set of changes, forked on Dec 14 in github @
https://github.com/elijahtaylor/mono. I would appreciate feedback on
these changes... to facilitate this, I'll try to explain the largest
changes
by feature (please email if clarification is needed):
*1) amd64 codegen*
- Rules located here:
http://www.chromium.org/nativeclient/design-documents/nacl-sfi-model-on-x86-64-systems
- Removed %r15 from register allocation, LMF save/restore, etc.
(r15 is special and not modifiable by untrusted code)
- Sandbox all data access through membase address mode. If not
%rsp or %rbp relative, re-write as clearing upper 32-bits + memindex
addressing
- align functions, call sites
- Sandbox returns and all indirect jumps (need to be 32-byte
aligned, cleared upper 32-bits)
- Never omit frame pointer as general operations to rbp aren't
allowed
*2) NaCl x86-64 is ILP32 (this is the largest set of changes and may
make some mono devs unhappy)*
- Set SIZEOF_REGISTER == 8 while sizeof(gpointer) == 4 for NaCl
amd64 (we can use 8-byte instructions, but pointers are 4-bytes)
- Re-write large portions of mini-amd64.c, tramp-amd64.c,
exceptions-amd64.c, mini.c, method-to-ir.c to use appropriate sizes
(SIZEOF_REGISTER, sizeof(gpointer), literal '8'). *These changes
are disruptive, but ultimately they should be more correct than what was
there before. *It's our opinion that these changes actually improve
Mono despite their impact.
- We only generate NaCl amd64 code from an ILP32 machine (either a
32-bit application for AOT code, or NaCl runtime JIT), so we may not
have
caught all of the [8 -- SIZEOF_REGISTER] conversions,
Re: [Mono-dev] [PATCH] more support for Google Native Client
Zoltan,
I've rebased from mono's master branch and fixed all merge conflicts, but
something that's gone in since I first forked has now broken NaCl AOT
compilation for me. On amd64 the compiler just crashes and I'm looking into
that, nut on x86 I'm getting this: Can't use AOT image 'mscorlib' in
aot-only mode because it is not compiled with --aot=full. But I'm compiling
with --aot=full,static,nodebug,ntrampolines=4096
If need be I can pick through the AOT changes that have gone in, but I was
hoping you or someone on this list would be able to tell me the major
changes to AOT from the past 3 weeks and some ideas about what might be
getting in my way. Can you shed any light?
-Elijah
On Wed, Jan 5, 2011 at 3:51 PM, Zoltan Varga var...@gmail.com wrote:
Hi,
I think the current code looks ok, and we should think about how to merge
it into mono trunk. As a first step, could you rebase your master branch on
top of master to fix the few conflicts which has surfaced due to changes to
mono master ?
Zoltan
On Wed, Jan 5, 2011 at 8:23 PM, Elijah Taylor elijahtay...@google.comwrote:
Hi Zoltan,
I've addressed all of the issues you pointed out (minus genmdesc.c:
__nacl_suspend_thread_if_needed, but that doesn't need to be merged in at
this time, it can remain in my local repository only). Please take another
look at your earliest convenience and let me know if there's anything else
you need from me.
-Elijah
On Tue, Jan 4, 2011 at 10:55 AM, Elijah Taylor
elijahtay...@google.comwrote:
Replies inline:
On Tue, Jan 4, 2011 at 10:30 AM, Zoltan Varga var...@gmail.com wrote:
Hi,
Some comments:
- the patch changes IMT_REG to AMD64_R11 in the non-nacl case, I'm not
sure thats
intentional.
Has this changed in the last six months on the Mono side? IIRC I didn't
mean to change anything like this. The reason I made explicit defines was
so code in aot-compiler and mini-amd64 could share defines over which reg
was the one we jump through and which was a scratch reg. I'll diff vs Mono
head revision and make it correct.
- you could define __mono_ilp32__ in the nacl/amd64 case, and use that
instead of
defined(__native_client_codegen__) defined(TARGET_AMD64) in a few
places.
That sounds reasonable. I'm assuming you mean non-arch specific areas
like mini.c, aot-*.c, method-to-ir.c, etc? Are there any other major
consequences to defining __mono_ilp32__ ?
- it would be better to define nacl_global_codeman_validate () as a
no-op in the non-nacl case, so its callers wouldn't need #ifdefs.
I'll fix this.
- genmdesc.c contains this change, which is probably not needed:
+void __nacl_suspend_thread_if_needed() {}
+
It is needed temporarily due to a preliminary GC implementation, we don't
have to submit it this way. Eventually (soon) we won't need it at all.
- you could use sizeof(mgreg_t) instead of SIZEOF_REGISTER to be
consistent with
the usage of sizeof(gpointer).
Sounds good. I'll try to use sizeof for all compiled code and only use
SIZEOF_REGISTER/SIZEOF_VOID_P for pre-processor directives only.
Other than these, I think the changes look fine, they aren't that
disruptive, since they don't
change the non-nacl behavior at all.
Great! I was worried just based on LOC changed that it might get more
resistance. In truth I'm more worried about future Mono changes
accidentally breaking NaCl behavior. I'm planning on getting some automated
testing implemented soon to combat this though.
On Tue, Dec 21, 2010 at 9:12 PM, Elijah Taylor elijahtay...@google.com
wrote:
Greetings Mono developers!
*[tl;dr very large patch for Native
Clienthttp://www.chromium.org/nativeclient support
hosted here https://github.com/elijahtaylor/mono, would love
feedback and many eyes to look at it]
*
I'm back with another round of changes for supporting Google's Native
Client (NaCl), including support for amd64, JIT compilation, and Garbage
Collection. It's a large set of changes, forked on Dec 14 in github @
https://github.com/elijahtaylor/mono. I would appreciate feedback on
these changes... to facilitate this, I'll try to explain the largest
changes
by feature (please email if clarification is needed):
*1) amd64 codegen*
- Rules located here:
http://www.chromium.org/nativeclient/design-documents/nacl-sfi-model-on-x86-64-systems
- Removed %r15 from register allocation, LMF save/restore, etc.
(r15 is special and not modifiable by untrusted code)
- Sandbox all data access through membase address mode. If not
%rsp or %rbp relative, re-write as clearing upper 32-bits + memindex
addressing
- align functions, call sites
- Sandbox returns and all indirect jumps (need to be 32-byte
aligned, cleared upper 32-bits)
- Never omit frame pointer as general operations to rbp aren't
allowed
*2) NaCl x86-64 is ILP32 (this is the largest set of
Re: [Mono-dev] [PATCH] more support for Google Native Client
Hi,
On Thu, Jan 6, 2011 at 1:24 AM, Elijah Taylor elijahtay...@google.comwrote:
Zoltan,
I've rebased from mono's master branch and fixed all merge conflicts, but
something that's gone in since I first forked has now broken NaCl AOT
compilation for me. On amd64 the compiler just crashes and I'm looking into
that, nut on x86 I'm getting this: Can't use AOT image 'mscorlib' in
aot-only mode because it is not compiled with --aot=full. But I'm
compiling with --aot=full,static,nodebug,ntrampolines=4096
If need be I can pick through the AOT changes that have gone in, but I was
hoping you or someone on this list would be able to tell me the major
changes to AOT from the past 3 weeks and some ideas about what might be
getting in my way. Can you shed any light?
There was a big reorganization in the AOT file format to reduce the number
of global symbols exported from the aot images. No idea why this is causing
problems. make fullaotcheck and make fsacheck still seems to work for me on
x86. I fixed a uninitilized memory error in 88d676ffd425def3, maybe that
will help.
Zoltan
-Elijah
On Wed, Jan 5, 2011 at 3:51 PM, Zoltan Varga var...@gmail.com wrote:
Hi,
I think the current code looks ok, and we should think about how to
merge it into mono trunk. As a first step, could you rebase your master
branch on top of master to fix the few conflicts which has surfaced due to
changes to mono master ?
Zoltan
On Wed, Jan 5, 2011 at 8:23 PM, Elijah Taylor elijahtay...@google.comwrote:
Hi Zoltan,
I've addressed all of the issues you pointed out (minus genmdesc.c:
__nacl_suspend_thread_if_needed, but that doesn't need to be merged in at
this time, it can remain in my local repository only). Please take another
look at your earliest convenience and let me know if there's anything else
you need from me.
-Elijah
On Tue, Jan 4, 2011 at 10:55 AM, Elijah Taylor
elijahtay...@google.comwrote:
Replies inline:
On Tue, Jan 4, 2011 at 10:30 AM, Zoltan Varga var...@gmail.com wrote:
Hi,
Some comments:
- the patch changes IMT_REG to AMD64_R11 in the non-nacl case, I'm not
sure thats
intentional.
Has this changed in the last six months on the Mono side? IIRC I didn't
mean to change anything like this. The reason I made explicit defines was
so code in aot-compiler and mini-amd64 could share defines over which reg
was the one we jump through and which was a scratch reg. I'll diff vs Mono
head revision and make it correct.
- you could define __mono_ilp32__ in the nacl/amd64 case, and use that
instead of
defined(__native_client_codegen__) defined(TARGET_AMD64) in a few
places.
That sounds reasonable. I'm assuming you mean non-arch specific areas
like mini.c, aot-*.c, method-to-ir.c, etc? Are there any other major
consequences to defining __mono_ilp32__ ?
- it would be better to define nacl_global_codeman_validate () as a
no-op in the non-nacl case, so its callers wouldn't need #ifdefs.
I'll fix this.
- genmdesc.c contains this change, which is probably not needed:
+void __nacl_suspend_thread_if_needed() {}
+
It is needed temporarily due to a preliminary GC implementation, we
don't have to submit it this way. Eventually (soon) we won't need it at
all.
- you could use sizeof(mgreg_t) instead of SIZEOF_REGISTER to be
consistent with
the usage of sizeof(gpointer).
Sounds good. I'll try to use sizeof for all compiled code and only use
SIZEOF_REGISTER/SIZEOF_VOID_P for pre-processor directives only.
Other than these, I think the changes look fine, they aren't that
disruptive, since they don't
change the non-nacl behavior at all.
Great! I was worried just based on LOC changed that it might get more
resistance. In truth I'm more worried about future Mono changes
accidentally breaking NaCl behavior. I'm planning on getting some
automated
testing implemented soon to combat this though.
On Tue, Dec 21, 2010 at 9:12 PM, Elijah Taylor
elijahtay...@google.com wrote:
Greetings Mono developers!
*[tl;dr very large patch for Native
Clienthttp://www.chromium.org/nativeclient support
hosted here https://github.com/elijahtaylor/mono, would love
feedback and many eyes to look at it]
*
I'm back with another round of changes for supporting Google's Native
Client (NaCl), including support for amd64, JIT compilation, and Garbage
Collection. It's a large set of changes, forked on Dec 14 in github @
https://github.com/elijahtaylor/mono. I would appreciate feedback on
these changes... to facilitate this, I'll try to explain the largest
changes
by feature (please email if clarification is needed):
*1) amd64 codegen*
- Rules located here:
http://www.chromium.org/nativeclient/design-documents/nacl-sfi-model-on-x86-64-systems
- Removed %r15 from register allocation, LMF save/restore, etc.
(r15 is special and not modifiable by untrusted
Re: [Mono-dev] [PATCH] more support for Google Native Client
Hi,
This should work as follows: every aot image contains a MonoAotFileInfo
structure,
emitted in emit_file_info () in aot-compiler.c, which has a 'flags' field,
and the MONO_AOT_FILE_FLAG_FULL_AOT flag should be set in
this field. At runtime, check_usable() in aot-runtime.c checks this flag.
Zoltan
On Thu, Jan 6, 2011 at 2:10 AM, Zoltan Varga var...@gmail.com wrote:
Hi,
On Thu, Jan 6, 2011 at 1:24 AM, Elijah Taylor elijahtay...@google.comwrote:
Zoltan,
I've rebased from mono's master branch and fixed all merge conflicts, but
something that's gone in since I first forked has now broken NaCl AOT
compilation for me. On amd64 the compiler just crashes and I'm looking into
that, nut on x86 I'm getting this: Can't use AOT image 'mscorlib' in
aot-only mode because it is not compiled with --aot=full. But I'm
compiling with --aot=full,static,nodebug,ntrampolines=4096
If need be I can pick through the AOT changes that have gone in, but I was
hoping you or someone on this list would be able to tell me the major
changes to AOT from the past 3 weeks and some ideas about what might be
getting in my way. Can you shed any light?
There was a big reorganization in the AOT file format to reduce the number
of global symbols exported from the aot images. No idea why this is causing
problems. make fullaotcheck and make fsacheck still seems to work for me on
x86. I fixed a uninitilized memory error in 88d676ffd425def3, maybe that
will help.
Zoltan
-Elijah
On Wed, Jan 5, 2011 at 3:51 PM, Zoltan Varga var...@gmail.com wrote:
Hi,
I think the current code looks ok, and we should think about how to
merge it into mono trunk. As a first step, could you rebase your master
branch on top of master to fix the few conflicts which has surfaced due to
changes to mono master ?
Zoltan
On Wed, Jan 5, 2011 at 8:23 PM, Elijah Taylor
elijahtay...@google.comwrote:
Hi Zoltan,
I've addressed all of the issues you pointed out (minus genmdesc.c:
__nacl_suspend_thread_if_needed, but that doesn't need to be merged in at
this time, it can remain in my local repository only). Please take another
look at your earliest convenience and let me know if there's anything else
you need from me.
-Elijah
On Tue, Jan 4, 2011 at 10:55 AM, Elijah Taylor elijahtay...@google.com
wrote:
Replies inline:
On Tue, Jan 4, 2011 at 10:30 AM, Zoltan Varga var...@gmail.comwrote:
Hi,
Some comments:
- the patch changes IMT_REG to AMD64_R11 in the non-nacl case, I'm not
sure thats
intentional.
Has this changed in the last six months on the Mono side? IIRC I
didn't mean to change anything like this. The reason I made explicit
defines was so code in aot-compiler and mini-amd64 could share defines
over
which reg was the one we jump through and which was a scratch reg. I'll
diff vs Mono head revision and make it correct.
- you could define __mono_ilp32__ in the nacl/amd64 case, and use that
instead of
defined(__native_client_codegen__) defined(TARGET_AMD64) in a few
places.
That sounds reasonable. I'm assuming you mean non-arch specific areas
like mini.c, aot-*.c, method-to-ir.c, etc? Are there any other major
consequences to defining __mono_ilp32__ ?
- it would be better to define nacl_global_codeman_validate () as a
no-op in the non-nacl case, so its callers wouldn't need #ifdefs.
I'll fix this.
- genmdesc.c contains this change, which is probably not needed:
+void __nacl_suspend_thread_if_needed() {}
+
It is needed temporarily due to a preliminary GC implementation, we
don't have to submit it this way. Eventually (soon) we won't need it at
all.
- you could use sizeof(mgreg_t) instead of SIZEOF_REGISTER to be
consistent with
the usage of sizeof(gpointer).
Sounds good. I'll try to use sizeof for all compiled code and only use
SIZEOF_REGISTER/SIZEOF_VOID_P for pre-processor directives only.
Other than these, I think the changes look fine, they aren't that
disruptive, since they don't
change the non-nacl behavior at all.
Great! I was worried just based on LOC changed that it might get more
resistance. In truth I'm more worried about future Mono changes
accidentally breaking NaCl behavior. I'm planning on getting some
automated
testing implemented soon to combat this though.
On Tue, Dec 21, 2010 at 9:12 PM, Elijah Taylor
elijahtay...@google.com wrote:
Greetings Mono developers!
*[tl;dr very large patch for Native
Clienthttp://www.chromium.org/nativeclient support
hosted here https://github.com/elijahtaylor/mono, would love
feedback and many eyes to look at it]
*
I'm back with another round of changes for supporting Google's Native
Client (NaCl), including support for amd64, JIT compilation, and Garbage
Collection. It's a large set of changes, forked on Dec 14 in github @
https://github.com/elijahtaylor/mono. I would appreciate
Re: [Mono-dev] [PATCH] more support for Google Native Client
Ok, I'll check out the changes/info you mentioned and go through the files
that auto-merged, too. Probably won't get this done for at least a day or
so, but I'll rebase again once I've fixed it. Hopefully by that point
something else won't have broken too :)
-Elijah
On Wed, Jan 5, 2011 at 5:19 PM, Zoltan Varga var...@gmail.com wrote:
Hi,
This should work as follows: every aot image contains a MonoAotFileInfo
structure,
emitted in emit_file_info () in aot-compiler.c, which has a 'flags' field,
and the MONO_AOT_FILE_FLAG_FULL_AOT flag should be set in
this field. At runtime, check_usable() in aot-runtime.c checks this flag.
Zoltan
On Thu, Jan 6, 2011 at 2:10 AM, Zoltan Varga var...@gmail.com wrote:
Hi,
On Thu, Jan 6, 2011 at 1:24 AM, Elijah Taylor elijahtay...@google.comwrote:
Zoltan,
I've rebased from mono's master branch and fixed all merge conflicts, but
something that's gone in since I first forked has now broken NaCl AOT
compilation for me. On amd64 the compiler just crashes and I'm looking into
that, nut on x86 I'm getting this: Can't use AOT image 'mscorlib' in
aot-only mode because it is not compiled with --aot=full. But I'm
compiling with --aot=full,static,nodebug,ntrampolines=4096
If need be I can pick through the AOT changes that have gone in, but I
was hoping you or someone on this list would be able to tell me the major
changes to AOT from the past 3 weeks and some ideas about what might be
getting in my way. Can you shed any light?
There was a big reorganization in the AOT file format to reduce the number
of global symbols exported from the aot images. No idea why this is causing
problems. make fullaotcheck and make fsacheck still seems to work for me on
x86. I fixed a uninitilized memory error in 88d676ffd425def3, maybe that
will help.
Zoltan
-Elijah
On Wed, Jan 5, 2011 at 3:51 PM, Zoltan Varga var...@gmail.com wrote:
Hi,
I think the current code looks ok, and we should think about how to
merge it into mono trunk. As a first step, could you rebase your master
branch on top of master to fix the few conflicts which has surfaced due to
changes to mono master ?
Zoltan
On Wed, Jan 5, 2011 at 8:23 PM, Elijah Taylor
elijahtay...@google.comwrote:
Hi Zoltan,
I've addressed all of the issues you pointed out (minus genmdesc.c:
__nacl_suspend_thread_if_needed, but that doesn't need to be merged in at
this time, it can remain in my local repository only). Please take
another
look at your earliest convenience and let me know if there's anything else
you need from me.
-Elijah
On Tue, Jan 4, 2011 at 10:55 AM, Elijah Taylor
elijahtay...@google.com wrote:
Replies inline:
On Tue, Jan 4, 2011 at 10:30 AM, Zoltan Varga var...@gmail.comwrote:
Hi,
Some comments:
- the patch changes IMT_REG to AMD64_R11 in the non-nacl case, I'm
not sure thats
intentional.
Has this changed in the last six months on the Mono side? IIRC I
didn't mean to change anything like this. The reason I made explicit
defines was so code in aot-compiler and mini-amd64 could share defines
over
which reg was the one we jump through and which was a scratch reg. I'll
diff vs Mono head revision and make it correct.
- you could define __mono_ilp32__ in the nacl/amd64 case, and use
that instead of
defined(__native_client_codegen__) defined(TARGET_AMD64) in a
few places.
That sounds reasonable. I'm assuming you mean non-arch specific areas
like mini.c, aot-*.c, method-to-ir.c, etc? Are there any other major
consequences to defining __mono_ilp32__ ?
- it would be better to define nacl_global_codeman_validate () as a
no-op in the non-nacl case, so its callers wouldn't need #ifdefs.
I'll fix this.
- genmdesc.c contains this change, which is probably not needed:
+void __nacl_suspend_thread_if_needed() {}
+
It is needed temporarily due to a preliminary GC implementation, we
don't have to submit it this way. Eventually (soon) we won't need it at
all.
- you could use sizeof(mgreg_t) instead of SIZEOF_REGISTER to be
consistent with
the usage of sizeof(gpointer).
Sounds good. I'll try to use sizeof for all compiled code and only
use SIZEOF_REGISTER/SIZEOF_VOID_P for pre-processor directives only.
Other than these, I think the changes look fine, they aren't that
disruptive, since they don't
change the non-nacl behavior at all.
Great! I was worried just based on LOC changed that it might get more
resistance. In truth I'm more worried about future Mono changes
accidentally breaking NaCl behavior. I'm planning on getting some
automated
testing implemented soon to combat this though.
On Tue, Dec 21, 2010 at 9:12 PM, Elijah Taylor
elijahtay...@google.com wrote:
Greetings Mono developers!
*[tl;dr very large patch for Native
Clienthttp://www.chromium.org/nativeclient support
hosted here
Re: [Mono-dev] [PATCH] more support for Google Native Client
Hi,
Some comments:
- the patch changes IMT_REG to AMD64_R11 in the non-nacl case, I'm not sure
thats
intentional.
- you could define __mono_ilp32__ in the nacl/amd64 case, and use that
instead of
defined(__native_client_codegen__) defined(TARGET_AMD64) in a few
places.
- it would be better to define nacl_global_codeman_validate () as a no-op in
the non-nacl case, so its callers wouldn't need #ifdefs.
- genmdesc.c contains this change, which is probably not needed:
+void __nacl_suspend_thread_if_needed() {}
+
- you could use sizeof(mgreg_t) instead of SIZEOF_REGISTER to be consistent
with
the usage of sizeof(gpointer).
Other than these, I think the changes look fine, they aren't that
disruptive, since they don't
change the non-nacl behavior at all.
Zoltan
On Tue, Dec 21, 2010 at 9:12 PM, Elijah Taylor elijahtay...@google.comwrote:
Greetings Mono developers!
*[tl;dr very large patch for Native
Clienthttp://www.chromium.org/nativeclient support
hosted here https://github.com/elijahtaylor/mono, would love feedback
and many eyes to look at it]
*
I'm back with another round of changes for supporting Google's Native
Client (NaCl), including support for amd64, JIT compilation, and Garbage
Collection. It's a large set of changes, forked on Dec 14 in github @
https://github.com/elijahtaylor/mono. I would appreciate feedback on
these changes... to facilitate this, I'll try to explain the largest changes
by feature (please email if clarification is needed):
*1) amd64 codegen*
- Rules located here:
http://www.chromium.org/nativeclient/design-documents/nacl-sfi-model-on-x86-64-systems
- Removed %r15 from register allocation, LMF save/restore, etc.
(r15 is special and not modifiable by untrusted code)
- Sandbox all data access through membase address mode. If not %rsp
or %rbp relative, re-write as clearing upper 32-bits + memindex
addressing
- align functions, call sites
- Sandbox returns and all indirect jumps (need to be 32-byte
aligned, cleared upper 32-bits)
- Never omit frame pointer as general operations to rbp aren't
allowed
*2) NaCl x86-64 is ILP32 (this is the largest set of changes and may make
some mono devs unhappy)*
- Set SIZEOF_REGISTER == 8 while sizeof(gpointer) == 4 for NaCl amd64
(we can use 8-byte instructions, but pointers are 4-bytes)
- Re-write large portions of mini-amd64.c, tramp-amd64.c,
exceptions-amd64.c, mini.c, method-to-ir.c to use appropriate sizes
(SIZEOF_REGISTER, sizeof(gpointer), literal '8'). *These changes are
disruptive, but ultimately they should be more correct than what was there
before. *It's our opinion that these changes actually improve Mono
despite their impact.
- We only generate NaCl amd64 code from an ILP32 machine (either a
32-bit application for AOT code, or NaCl runtime JIT), so we may not have
caught all of the [8 -- SIZEOF_REGISTER] conversions, but we likely
caught
most of the [sizeof(gpointer) -- SIZEOF_REGISTER] and [8 --
sizeof(gpointer)] changes that are necessary.
- Change atomic operations and default pointer directives to use 32-bit
instructions (long instead of quad)
- Change default operations to use 32-bit integers/pointers (eg,
OP_LOAD_MEMBASE uses 4-bytes instead of 8)
*3) JIT support for NaCl*
- Since we're unable to emit code directly in its final executable
location, we instead:
- reserve a buffer on the heap
- create a hash table entry mapping the temp location and final
location
- modify all non-local patches relative to the final location
- request the NaCl runtime to install the created code in the final
location
- See mono/utils/mono-codeman.c changes for more detail.
- For every codeman *reserve*, we must add a codeman *validate* call in
order to install the method/trampoline/blob in the final location (as well
as validate it for NaCl, pad it out, etc)
- We don't delete or reuse code (we can, but it's icky and the
benefits don't outweigh the cost)
- Backpatching changed to use NaCl syscalls to modify existing dynamic
code
*4) GC support for NaCl (boehm only)*
- NaCl compiler and Mono code generator both emit instrumentation at GC
safe points (back branches and function prologs), for cooperative thread
parking (we're not allowed to send and receive signals)
- Added new opcode OP_NACL_GC_SAFE_POINT to handle mono instrumentation
- modified pthread_stop_world.c and pthread_support.c somewhat
extensively to support this new way of stopping the world
- wrapped pthread_exit because NaCl doesn't support pthread cleanup
functions
- added machine type NACL to libgc with machine specific defines
*5) Misc bug fixes (not NaCl-specific)*
- fix *x86_memindex_emit* when disp is 32-bit
Re: [Mono-dev] [PATCH] more support for Google Native Client
Replies inline:
On Tue, Jan 4, 2011 at 10:30 AM, Zoltan Varga var...@gmail.com wrote:
Hi,
Some comments:
- the patch changes IMT_REG to AMD64_R11 in the non-nacl case, I'm not sure
thats
intentional.
Has this changed in the last six months on the Mono side? IIRC I didn't
mean to change anything like this. The reason I made explicit defines was
so code in aot-compiler and mini-amd64 could share defines over which reg
was the one we jump through and which was a scratch reg. I'll diff vs Mono
head revision and make it correct.
- you could define __mono_ilp32__ in the nacl/amd64 case, and use that
instead of
defined(__native_client_codegen__) defined(TARGET_AMD64) in a few
places.
That sounds reasonable. I'm assuming you mean non-arch specific areas like
mini.c, aot-*.c, method-to-ir.c, etc? Are there any other major
consequences to defining __mono_ilp32__ ?
- it would be better to define nacl_global_codeman_validate () as a no-op
in the non-nacl case, so its callers wouldn't need #ifdefs.
I'll fix this.
- genmdesc.c contains this change, which is probably not needed:
+void __nacl_suspend_thread_if_needed() {}
+
It is needed temporarily due to a preliminary GC implementation, we don't
have to submit it this way. Eventually (soon) we won't need it at all.
- you could use sizeof(mgreg_t) instead of SIZEOF_REGISTER to be consistent
with
the usage of sizeof(gpointer).
Sounds good. I'll try to use sizeof for all compiled code and only use
SIZEOF_REGISTER/SIZEOF_VOID_P for pre-processor directives only.
Other than these, I think the changes look fine, they aren't that
disruptive, since they don't
change the non-nacl behavior at all.
Great! I was worried just based on LOC changed that it might get more
resistance. In truth I'm more worried about future Mono changes
accidentally breaking NaCl behavior. I'm planning on getting some automated
testing implemented soon to combat this though.
On Tue, Dec 21, 2010 at 9:12 PM, Elijah Taylor elijahtay...@google.comwrote:
Greetings Mono developers!
*[tl;dr very large patch for Native
Clienthttp://www.chromium.org/nativeclient support
hosted here https://github.com/elijahtaylor/mono, would love feedback
and many eyes to look at it]
*
I'm back with another round of changes for supporting Google's Native
Client (NaCl), including support for amd64, JIT compilation, and Garbage
Collection. It's a large set of changes, forked on Dec 14 in github @
https://github.com/elijahtaylor/mono. I would appreciate feedback on
these changes... to facilitate this, I'll try to explain the largest changes
by feature (please email if clarification is needed):
*1) amd64 codegen*
- Rules located here:
http://www.chromium.org/nativeclient/design-documents/nacl-sfi-model-on-x86-64-systems
- Removed %r15 from register allocation, LMF save/restore, etc.
(r15 is special and not modifiable by untrusted code)
- Sandbox all data access through membase address mode. If not
%rsp or %rbp relative, re-write as clearing upper 32-bits + memindex
addressing
- align functions, call sites
- Sandbox returns and all indirect jumps (need to be 32-byte
aligned, cleared upper 32-bits)
- Never omit frame pointer as general operations to rbp aren't
allowed
*2) NaCl x86-64 is ILP32 (this is the largest set of changes and may make
some mono devs unhappy)*
- Set SIZEOF_REGISTER == 8 while sizeof(gpointer) == 4 for NaCl amd64
(we can use 8-byte instructions, but pointers are 4-bytes)
- Re-write large portions of mini-amd64.c, tramp-amd64.c,
exceptions-amd64.c, mini.c, method-to-ir.c to use appropriate sizes
(SIZEOF_REGISTER, sizeof(gpointer), literal '8'). *These changes are
disruptive, but ultimately they should be more correct than what was there
before. *It's our opinion that these changes actually improve Mono
despite their impact.
- We only generate NaCl amd64 code from an ILP32 machine (either a
32-bit application for AOT code, or NaCl runtime JIT), so we may not have
caught all of the [8 -- SIZEOF_REGISTER] conversions, but we likely
caught
most of the [sizeof(gpointer) -- SIZEOF_REGISTER] and [8 --
sizeof(gpointer)] changes that are necessary.
- Change atomic operations and default pointer directives to use
32-bit instructions (long instead of quad)
- Change default operations to use 32-bit integers/pointers (eg,
OP_LOAD_MEMBASE uses 4-bytes instead of 8)
*3) JIT support for NaCl*
- Since we're unable to emit code directly in its final executable
location, we instead:
- reserve a buffer on the heap
- create a hash table entry mapping the temp location and final
location
- modify all non-local patches relative to the final location
- request the NaCl runtime to install the created code in the final
location
- See
Re: [Mono-dev] [PATCH] more support for Google Native Client
Hi Elijah, The patchset is indeed huge and will take quite some review effort to get it in. I'll start by reviewing the bug fixes then move to ILP32 support. Since you guys had to add GC support for cooperative parking, it would be interesting to merge this effort with the current one toward adding it to sgen, which has received quite some reviewing and testing. Mark Probst can better talk about it once he gets back from his vacations. One problem I see in the general description of how parking is been done is that it doesn't handle blocking syscalls, which might lead to a single thread make the GC halt. I'll start reviewing/cherry picking patches next year once I get back to work. Just wanted to give you a heads up. On Tue, Dec 21, 2010 at 6:12 PM, Elijah Taylor elijahtay...@google.comwrote: Greetings Mono developers! *[tl;dr very large patch for Native Clienthttp://www.chromium.org/nativeclient support hosted here https://github.com/elijahtaylor/mono, would love feedback and many eyes to look at it] * I'm back with another round of changes for supporting Google's Native Client (NaCl), including support for amd64, JIT compilation, and Garbage Collection. It's a large set of changes, forked on Dec 14 in github @ https://github.com/elijahtaylor/mono. I would appreciate feedback on these changes... to facilitate this, I'll try to explain the largest changes by feature (please email if clarification is needed): *1) amd64 codegen* - Rules located here: http://www.chromium.org/nativeclient/design-documents/nacl-sfi-model-on-x86-64-systems - Removed %r15 from register allocation, LMF save/restore, etc. (r15 is special and not modifiable by untrusted code) - Sandbox all data access through membase address mode. If not %rsp or %rbp relative, re-write as clearing upper 32-bits + memindex addressing - align functions, call sites - Sandbox returns and all indirect jumps (need to be 32-byte aligned, cleared upper 32-bits) - Never omit frame pointer as general operations to rbp aren't allowed *2) NaCl x86-64 is ILP32 (this is the largest set of changes and may make some mono devs unhappy)* - Set SIZEOF_REGISTER == 8 while sizeof(gpointer) == 4 for NaCl amd64 (we can use 8-byte instructions, but pointers are 4-bytes) - Re-write large portions of mini-amd64.c, tramp-amd64.c, exceptions-amd64.c, mini.c, method-to-ir.c to use appropriate sizes (SIZEOF_REGISTER, sizeof(gpointer), literal '8'). *These changes are disruptive, but ultimately they should be more correct than what was there before. *It's our opinion that these changes actually improve Mono despite their impact. - We only generate NaCl amd64 code from an ILP32 machine (either a 32-bit application for AOT code, or NaCl runtime JIT), so we may not have caught all of the [8 -- SIZEOF_REGISTER] conversions, but we likely caught most of the [sizeof(gpointer) -- SIZEOF_REGISTER] and [8 -- sizeof(gpointer)] changes that are necessary. - Change atomic operations and default pointer directives to use 32-bit instructions (long instead of quad) - Change default operations to use 32-bit integers/pointers (eg, OP_LOAD_MEMBASE uses 4-bytes instead of 8) *3) JIT support for NaCl* - Since we're unable to emit code directly in its final executable location, we instead: - reserve a buffer on the heap - create a hash table entry mapping the temp location and final location - modify all non-local patches relative to the final location - request the NaCl runtime to install the created code in the final location - See mono/utils/mono-codeman.c changes for more detail. - For every codeman *reserve*, we must add a codeman *validate* call in order to install the method/trampoline/blob in the final location (as well as validate it for NaCl, pad it out, etc) - We don't delete or reuse code (we can, but it's icky and the benefits don't outweigh the cost) - Backpatching changed to use NaCl syscalls to modify existing dynamic code *4) GC support for NaCl (boehm only)* - NaCl compiler and Mono code generator both emit instrumentation at GC safe points (back branches and function prologs), for cooperative thread parking (we're not allowed to send and receive signals) - Added new opcode OP_NACL_GC_SAFE_POINT to handle mono instrumentation - modified pthread_stop_world.c and pthread_support.c somewhat extensively to support this new way of stopping the world - wrapped pthread_exit because NaCl doesn't support pthread cleanup functions - added machine type NACL to libgc with machine specific defines *5) Misc bug fixes (not NaCl-specific)* - fix *x86_memindex_emit* when disp is 32-bit - properly exclude code in libgc/gc_dlopen.c when DYNAMIC_LOADING not defined - properly
Re: [Mono-dev] [PATCH] more support for Google Native Client
Hi Rodrigo, On Sat, Dec 25, 2010 at 5:01 AM, Rodrigo Kumpera kump...@gmail.com wrote: The patchset is indeed huge and will take quite some review effort to get it in. I'll start by reviewing the bug fixes then move to ILP32 support. Yes, sorry about that. I would have loved to have sent more targetted changes, but there were always gotchas with a lot of stuff not working until the whole system worked. No rush on getting this checked in, it's your codebase. Since you guys had to add GC support for cooperative parking, it would be interesting to merge this effort with the current one toward adding it to sgen, which has received quite some reviewing and testing. Mark Probst can better talk about it once he gets back from his vacations. I would very much love to get GC working with Sgen, but haven't put in the effort yet. We were working off of a snapshot from earlier this year prior to 2.8 so we just had boehm mature enough to work on. One problem I see in the general description of how parking is been done is that it doesn't handle blocking syscalls, which might lead to a single thread make the GC halt. I glazed over the details a little coarsely, but we do handle this case. All OS syscalls in Native Client are handled by the trusted runtime component of our system. Any time we issue a Native Client syscall (move from untrusted code to trusted code) that has a chance of blocking I store a context and count that thread as parked (threads in trusted code can't permute untrusted GC memory). Upon returning from trusted code I check if we're in the middle of GC and if so actually park until it's done. For details you can check libgc/pthread_stop_world.c, search for syscall and you can see the enter and exit hooks I've exposed from our syscall interface. I'll start reviewing/cherry picking patches next year once I get back to work. Just wanted to give you a heads up. Great, ping me if you have any questions or concerns and I'll try to get back to you quickly. I'm on vacation off and on until after the new year, but I'll have email access most of the time. -Elijah ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
[Mono-dev] [PATCH] more support for Google Native Client
Greetings Mono developers! *[tl;dr very large patch for Native Clienthttp://www.chromium.org/nativeclient support hosted here https://github.com/elijahtaylor/mono, would love feedback and many eyes to look at it] * I'm back with another round of changes for supporting Google's Native Client (NaCl), including support for amd64, JIT compilation, and Garbage Collection. It's a large set of changes, forked on Dec 14 in github @ https://github.com/elijahtaylor/mono. I would appreciate feedback on these changes... to facilitate this, I'll try to explain the largest changes by feature (please email if clarification is needed): *1) amd64 codegen* - Rules located here: http://www.chromium.org/nativeclient/design-documents/nacl-sfi-model-on-x86-64-systems - Removed %r15 from register allocation, LMF save/restore, etc. (r15 is special and not modifiable by untrusted code) - Sandbox all data access through membase address mode. If not %rsp or %rbp relative, re-write as clearing upper 32-bits + memindex addressing - align functions, call sites - Sandbox returns and all indirect jumps (need to be 32-byte aligned, cleared upper 32-bits) - Never omit frame pointer as general operations to rbp aren't allowed *2) NaCl x86-64 is ILP32 (this is the largest set of changes and may make some mono devs unhappy)* - Set SIZEOF_REGISTER == 8 while sizeof(gpointer) == 4 for NaCl amd64 (we can use 8-byte instructions, but pointers are 4-bytes) - Re-write large portions of mini-amd64.c, tramp-amd64.c, exceptions-amd64.c, mini.c, method-to-ir.c to use appropriate sizes (SIZEOF_REGISTER, sizeof(gpointer), literal '8'). *These changes are disruptive, but ultimately they should be more correct than what was there before. *It's our opinion that these changes actually improve Mono despite their impact. - We only generate NaCl amd64 code from an ILP32 machine (either a 32-bit application for AOT code, or NaCl runtime JIT), so we may not have caught all of the [8 -- SIZEOF_REGISTER] conversions, but we likely caught most of the [sizeof(gpointer) -- SIZEOF_REGISTER] and [8 -- sizeof(gpointer)] changes that are necessary. - Change atomic operations and default pointer directives to use 32-bit instructions (long instead of quad) - Change default operations to use 32-bit integers/pointers (eg, OP_LOAD_MEMBASE uses 4-bytes instead of 8) *3) JIT support for NaCl* - Since we're unable to emit code directly in its final executable location, we instead: - reserve a buffer on the heap - create a hash table entry mapping the temp location and final location - modify all non-local patches relative to the final location - request the NaCl runtime to install the created code in the final location - See mono/utils/mono-codeman.c changes for more detail. - For every codeman *reserve*, we must add a codeman *validate* call in order to install the method/trampoline/blob in the final location (as well as validate it for NaCl, pad it out, etc) - We don't delete or reuse code (we can, but it's icky and the benefits don't outweigh the cost) - Backpatching changed to use NaCl syscalls to modify existing dynamic code *4) GC support for NaCl (boehm only)* - NaCl compiler and Mono code generator both emit instrumentation at GC safe points (back branches and function prologs), for cooperative thread parking (we're not allowed to send and receive signals) - Added new opcode OP_NACL_GC_SAFE_POINT to handle mono instrumentation - modified pthread_stop_world.c and pthread_support.c somewhat extensively to support this new way of stopping the world - wrapped pthread_exit because NaCl doesn't support pthread cleanup functions - added machine type NACL to libgc with machine specific defines *5) Misc bug fixes (not NaCl-specific)* - fix *x86_memindex_emit* when disp is 32-bit - properly exclude code in libgc/gc_dlopen.c when DYNAMIC_LOADING not defined - properly exclude code based on DISABLE_SOCKETS by including config.h before checking define - clean up calculation of offset for amd64 AOT specific trampoline args - fix bug in *mono_bblock_insert_before_ins* when trying to insert an instruction to the beginning of an existing basic block. - fix small typo bug in genmdesc.pl which kept amd64 from being able to be a target of cross compiling - fix struct passing in amd64 with sizeof(struct) == 16 when fields aren't 8-byte aligned (eg, first field is 12 bytes, second field is 4 bytes), pass on stack instead of in registers (mini-amd64.c:* add_valuetype*) - add extra checks to mini-amd64.c:*mono_arch_emit_exceptions* to keep exception/R4/R8 emitting from overflowing a buffer silently - fix bugs in *new_codechunk* and *mono_code_manager_reserve_align* which allowed unaligned code to be allocated. I know
