Skip to content

Conversation

@atrosinenko
Copy link
Contributor

Fixes #4837. An Emscripten implementation of the pwrite64 system call is untested and references the wrong variable. This pull request fixes crash when this syscall is invoked and adds some tests.

The test_unistd_io passes.
There were 100 random tests run and all of them passed:

  • asm1.test_simd7
  • asm2f.test_pack
  • asm3.test_cases
  • asm2g.test_reinterpreted_ptrs
  • asm3.test_bad_typeid
  • asm2g.test_stdbool
  • asm2i.test_sizeof
  • asm2nn.test_simd_float64x2
  • asm2f.test_stack_overflow
  • asm2nn.test_sse4_1_full
  • asm2.test_vprintf
  • asm2nn.test_i64_cmp
  • asm2i.test_sscanf_n
  • asm2nn.test_asmjs_unknown_emscripten
  • asm2nn.test_sscanf
  • asm3.test_align64
  • asm2f.test_mainenv
  • asm3.test_sscanf_n
  • asm3.test_mathfuncptr
  • asm1.test_simd_int32x4
  • asm2.test_i64_2
  • asm2.test_em_asm_unused_arguments
  • asm2nn.test_exceptions_uncaught_2
  • asm2.test_dlfcn_mallocs
  • asm2g.test_i64_llabs
  • asm2g.test_dylink_printfs
  • default.test_fs_mmap
  • asm2nn.test_openjpeg
  • asm2f.test_sscanf_3
  • asm2f.test_isdigit_l
  • asm3.test_errar
  • asm1.test_embind_2
  • asm2nn.test_iswdigit
  • asm1.test_negative_zero
  • asm1.test_exceptions_convert
  • asm3.test_simd16
  • asm2.test_strtoll_dec
  • asm2f.test_structs
  • asm2nn.test_llvm_used
  • asm1.test_iswdigit
  • asm2g.test_python
  • asm2.test_printf_float
  • asm2f.test_exceptions_libcxx
  • asm2i.test_openjpeg
  • asm2i.test_fgetc_unsigned
  • asm2g.test_strtod
  • default.test_ssr
  • asm2g.test_getgep
  • asm3.test_math
  • asm2.test_phiundef
  • asm2.test_funcptr
  • asm2.test_exceptions
  • asm2f.test_dlfcn_i64
  • asm3.test_lua
  • asm1.test_libcextra
  • asm2nn.test_simd3
  • default.test_isdigit_l
  • default.test_em_asm_parameter_pack
  • asm2f.test_newstruct
  • asm2nn.test_isnan
  • asm2i.test_closebitcasts
  • asm2g.test_math_lgamma
  • asm2nn.test_atomic_cxx
  • asm2.test_mmap_file
  • asm2g.test_pack
  • asm2nn.test_assert
  • asm1.test_sscanf_4
  • asm2i.test_align64
  • asm1.test_dylink_jslib
  • asm2i.test_mmap
  • asm1.test_alloca_stack
  • asm2g.test_memmove3
  • asm2f.test_intentional_fault
  • asm3.test_raytrace
  • asm2f.test_polymorph
  • asm2g.test_exceptions_convert
  • default.test_fs_base
  • asm2.test_dlfcn_i64
  • asm1.test_bad_typeid
  • asm2f.test_exceptions_uncaught
  • asm2f.test_zlib
  • asm2i.test_uname
  • default.test_sbrk
  • default.test_literal_negative_zero
  • default.test_dylink_global_init
  • asm2nn.test_exit_status
  • asm2f.test_stat
  • asm2.test_unistd_dup
  • asm3.test_bitfields
  • asm2f.test_stdbool
  • asm1.test_stack_overflow_check
  • default.test_dylink_global_inits
  • asm2nn.test_binaryen
  • asm3.test_dead_functions
  • asm3.test_dlfcn_longjmp
  • asm3.test_strptime_reentrant
  • asm2.test_double_i64_conversion
  • asm2.test_emptyclass
  • asm2f.test_statvfs
  • asm2i.test_simd4

There was an implementation of the `pwrite64` syscall in Emscripten,
but when invoked it crashed because of ReferenceError.

This commit replaces the `nbyte` variable with the `count` one and
adds some tests for this syscall.
@kripken
Copy link
Member

kripken commented Jan 10, 2017

Thanks, this looks great, except I have a question about the test. It seems like it should be added 5 characters, but instead the read size after is increased by 6. And also the appended text at the end is \0 + write instead of just write, so that \0 may be the extra character, that I can't figure out where it's from?

@atrosinenko
Copy link
Contributor Author

I supposed that extra \0 before write is due to offset = 32. It seems logically: if I would specify offset = 0, it skips 0 bytes and writes at the beginning, so when I specify offset = 32, it skips 32 bytes (only 31 is available, so it adds extra \0 -- I didn't noticed any direct notes about that in the pwrite man page, but there are some phrases about lseek + write behaving that way on the lseek man page) and writes the write string, so final read size if 32 + 5.

@kripken
Copy link
Member

kripken commented Jan 11, 2017

Oh, I see, yeah. I thought there were 32 characters before, that confused me.

@kripken kripken merged commit 2634164 into emscripten-core:incoming Jan 11, 2017
@kripken
Copy link
Member

kripken commented Jan 11, 2017

Thanks!

@atrosinenko atrosinenko deleted the issue-pwrite64 branch January 14, 2017 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pwrite64 syscall crashes at run time

2 participants