Hi Michael, I noticed the break. After I tried to revert all my changes around mmap(), the test was still broken as the same as yours.
Looked into the issue further, I kept my change untouched and reverted two changes submitted between my changes as listed below. All tests were successful afterwards. I am not very familiar with tcc code base yet, but it seems that it is more suspicious that the changes below broke the tests, instead of the mmap() one? http://repo.or.cz/w/tinycc.git/commit/fdf9fba5785f5c0d285c8cbf2fa51df32ddd878d http://repo.or.cz/w/tinycc.git/commit/ea7b17f641cb962d0e9a79137e93c7e1e24b99ce Revert all mmap() changes #################### ------------ test3 ------------ ../tcc -B.. -I.. -I.. -I../include -DCONFIG_LDDIR="\"lib64\"" -DTCC_TARGET_X86_64 -DONE_SOURCE -run ../tcc.c -B.. -I.. -I.. -I../include -DCONFIG_LDDIR="\"lib64\"" -DTCC_TARGET_X86_64 -DONE_SOURCE -run ../tcc.c -B.. -I.. -I.. -I../include -DCONFIG_LDDIR="\"lib64\"" -DTCC_TARGET_X86_64 -DONE_SOURCE -run ../tcc.c -B.. -I.. -I.. -I../include -run tcctest.c > test.out3 /bin/sh: line 1: 3026 Segmentation fault (core dumped) ../tcc -B.. -I.. -I.. -I../include -DCONFIG_LDDIR="\"lib64\"" -DTCC_TARGET_X86_64 -DONE_SOURCE -run ../tcc.c -B.. -I.. -I.. -I../include -DCONFIG_LDDIR="\"lib64\"" -DTCC_TARGET_X86_64 -DONE_SOURCE -run ../tcc.c -B.. -I.. -I.. -I../include -DCONFIG_LDDIR="\"lib64\"" -DTCC_TARGET_X86_64 -DONE_SOURCE -run ../tcc.c -B.. -I.. -I.. -I../include -run tcctest.c > test.out3 Makefile:123: recipe for target 'test3' failed make[1]: *** [test3] Error 139 make[1]: Leaving directory '/home/keren/mob/tinycc/tests' Makefile:350: recipe for target 'test' failed make: *** [test] Error 2 Revert two changes listed above about fixing PE x86_64, while keeping mmap() change ##################### ------------ test3 ------------ ../tcc -B.. -I.. -I.. -I../include -DCONFIG_LDDIR="\"lib64\"" -DTCC_TARGET_X86_64 -DONE_SOURCE -run ../tcc.c -B.. -I.. -I.. -I../include -DCONFIG_LDDIR="\"lib64\"" -DTCC_TARGET_X86_64 -DONE_SOURCE -run ../tcc.c -B.. -I.. -I.. -I../include -DCONFIG_LDDIR="\"lib64\"" -DTCC_TARGET_X86_64 -DONE_SOURCE -run ../tcc.c -B.. -I.. -I.. -I../include -run tcctest.c > test.out3 Auto Test3 OK gcc -o abitest-cc abitest.c ../libtcc.a -I.. -Wall -g -O2 -fno-strict-aliasing -Wno-pointer-sign -Wno-sign-compare -Wno-unused-result -DCONFIG_LDDIR="\"lib64\"" -DTCC_TARGET_X86_64 -lm -ldl -I.. ../tcc -B.. -I.. -I.. -I../include -o abitest-tcc abitest.c ../libtcc.a -I.. -Wall -g -O2 -fno-strict-aliasing -Wno-pointer-sign -Wno-sign-compare -Wno-unused-result -DCONFIG_LDDIR="\"lib64\"" -DTCC_TARGET_X86_64 -lm -ldl -I.. ------------ abitest ------------ ./abitest-cc lib_path=.. include="../include" ret_int_test... success ret_longlong_test... success ret_float_test... success ret_double_test... success ret_longdouble_test... success ret_2float_test... success ret_2double_test... success reg_pack_test... success reg_pack_longlong_test... success sret_test... success one_member_union_test... success two_member_union_test... success many_struct_test... success many_struct_test_2... success stdarg_test... success stdarg_struct_test... success arg_align_test... success ./abitest-tcc lib_path=.. include="../include" ret_int_test... success ret_longlong_test... success ret_float_test... success ret_double_test... success ret_longdouble_test... success ret_2float_test... success ret_2double_test... success reg_pack_test... success reg_pack_longlong_test... success sret_test... success one_member_union_test... success two_member_union_test... success many_struct_test... success many_struct_test_2... success stdarg_test... success stdarg_struct_test... success arg_align_test... success ../tcc -B.. -I.. -I.. -I../include -o vla_test vla_test.c -I.. -Wall -g -O2 -fno-strict-aliasing -Wno-pointer-sign -Wno-sign-compare -Wno-unused-result ------------ vla_test-run ------------ ./vla_test test1... success test2... success test3... success ------------ moretests ------------ make -C tests2 make[2]: Entering directory '/home/keren/mob/tinycc/tests/tests2' Test: 00_assignment... Test: 01_comment... Test: 02_printf... Test: 03_struct... Test: 04_for... Test: 05_array... Test: 06_case... Test: 07_function... Test: 08_while... Test: 09_do_while... Test: 10_pointer... Test: 11_precedence... Test: 12_hashdefine... Test: 13_integer_literals... Test: 14_if... Test: 15_recursion... Test: 16_nesting... Test: 17_enum... Test: 18_include... Test: 19_pointer_arithmetic... Test: 20_pointer_comparison... Test: 21_char_array... Test: 22_floating_point... Test: 23_type_coercion... Test: 24_math_library... Test: 25_quicksort... Test: 26_character_constants... Test: 27_sizeof... Test: 28_strings... Test: 29_array_address... Test: 31_args... Test: 32_led... Test: 33_ternary_op... Test: 35_sizeof... Test: 36_array_initialisers... Test: 37_sprintf... Test: 38_multiple_array_index... Test: 39_typedef... Test: 40_stdio... Test: 41_hashif... Test: 42_function_pointer... Test: 43_void_param... Test: 44_scoped_declarations... Test: 45_empty_for... Test: 47_switch_return... Test: 48_nested_break... Test: 49_bracket_evaluation... Test: 50_logical_second_arg... Test: 51_static... Test: 52_unnamed_enum... Test: 54_goto... Test: 55_lshift_type... make[2]: Leaving directory '/home/keren/mob/tinycc/tests/tests2' make[1]: Leaving directory '/home/keren/mob/tinycc/tests' And my reverted diff ################## [keren@speedbird tinycc]$ git diff diff --git a/tccgen.c b/tccgen.c index c7f0a87..f5ebcc5 100644 --- a/tccgen.c +++ b/tccgen.c @@ -1955,9 +1955,6 @@ static void gen_cast(CType *type) s = 24; else if ((dbt & VT_BTYPE) == VT_SHORT) s = 16; -#ifdef TCC_TARGET_X86_64 - if (!(dbt & (VT_PTR|VT_LLONG|VT_FUNC|VT_STRUCT))) -#endif if(dbt & VT_UNSIGNED) vtop->c.ui = ((unsigned int)vtop->c.ll << s) >> s; else @@ -3908,11 +3905,7 @@ ST_FUNC void unary(void) /* if forward reference, we must point to s */ if (vtop->r & VT_SYM) { vtop->sym = s; -#ifdef TCC_TARGET_X86_64 - vtop->c.ull = 0; -#else vtop->c.ul = 0; -#endif } break; } @@ -5126,12 +5119,6 @@ static void init_putv(CType *type, Section *sec, unsigned long c, case VT_LLONG: *(long long *)ptr |= (vtop->c.ll & bit_mask) << bit_pos; break; - case VT_PTR: - if (vtop->r & VT_SYM) { - greloc(sec, vtop->sym, c, R_DATA_PTR); - } - *(addr_t *)ptr |= (vtop->c.ull & bit_mask) << bit_pos; - break; default: if (vtop->r & VT_SYM) { greloc(sec, vtop->sym, c, R_DATA_PTR); Best, Keren On Sat, Jan 11, 2014 at 10:41 AM, Michael Matz <[email protected]> wrote: > Hi, > > > On Thu, 9 Jan 2014, Keren Tan wrote: > > I just submitted a tentative patch to the mob branch about mmap. When >> selinux is enabled, tccrun.c uses mmap to hold the dynamically generated >> code/data. It is backed by a randomly named file under /tmp directory. My >> patch is to use an anonymous file in mmap instead, so that the generated >> code/data only resides in memory, and tcc does not depend on a writable >> /tmp anymore. >> > > It's customary to actually test changes before committing. In your case: > > % ./configure --with-selinux > % make && make test > ... > ------------ test3 ------------ > ../tcc -B.. -I.. -I.. -I../include -DCONFIG_LDDIR="\"lib64\"" > -DTCC_TARGET_X86_64 -DONE_SOURCE -run ../tcc.c -B.. -I.. -I.. -I../include > -DCONFIG_LDDIR="\"lib64\"" -DTCC_TARGET_X86_64 -DONE_SOURCE -run ../tcc.c > -B.. -I.. -I.. -I../include -DCONFIG_LDDIR="\"lib64\"" -DTCC_TARGET_X86_64 > -DONE_SOURCE -run ../tcc.c -B.. -I.. -I.. -I../include -run tcctest.c > > test.out3 > /bin/sh: line 1: 15954 Segmentation fault ../tcc -B.. -I.. -I.. > -I../include -DCONFIG_LDDIR="\"lib64\"" -DTCC_TARGET_X86_64 -DONE_SOURCE > -run ../tcc.c -B.. -I.. -I.. -I../include -DCONFIG_LDDIR="\"lib64\"" > -DTCC_TARGET_X86_64 -DONE_SOURCE -run ../tcc.c -B.. -I.. -I.. -I../include > -DCONFIG_LDDIR="\"lib64\"" -DTCC_TARGET_X86_64 -DONE_SOURCE -run ../tcc.c > -B.. -I.. -I.. -I../include -run tcctest.c > test.out3 > make[1]: *** [test3] Error 139 > > This is no wonder because the former mmap mechanism had some special > requirements mandated by SElinux that you ignored. Your patch basically > did this: > > s1->write_mem = mmap (NULL, ret, PROT_READ|PROT_WRITE, > - MAP_SHARED, fd, 0); > + MAP_PRIVATE|MAP_ANONYMOUS, -1, 0); > ... > s1->runtime_mem = mmap (NULL, ret, PROT_READ|PROT_EXEC, > - MAP_SHARED, fd, 0); > + MAP_PRIVATE|MAP_ANONYMOUS, -1, 0); > > So it replaced two mmaps of the same region (the file), one mapping being > writable (pointer to it in ->write_mem), and another mapping being > executable (pointer in ->runtime_mem) by two completely unrelated anon > mappings, one writable, one executable. > > The requirements of the whole things are such, that changes to the > writable mapping via ->write_mem need to be reflected in the mapping > reachable via ->runtime_mem. I.e. both mappings really need to be two > views on _exactly_ the same data. > > Depending on the configuration of an SElinux system the following things > might be disallowed: > * creating mapping which is WRITE|EXEC > * changing flags of an existing writable mapping to include EXEC when it > didn't before (no matter if WRITE is now removed or not) > > The first means you won't get away with just a single mmap of some > ANON|PRIVATE that is writable and executable. The second means you also > don't get away with first mapping something writable, let tcc generate its > code therein and then remap it executable (either via mprotect or mremap). > That is you indeed need two separate mappings. And because both mappings > need to be actually from the same underlying data they need to be related > somehow. And the only way to relate two separate mappings is via a file > descriptor and a MAP_SHARED mapping. > > IOW: the former way of mapping a writable and executable piece of memory > via a shared mapping backed by a file is the only way that works in > SElinux. And yes, that means it needs to have a place for that file, and > it needs to be on a file system that isn't mounted noexec (i.e. /tmp might > actually not be the right place). > > In still other words: please revert, sorry. :-/ > > > tcc is fantastic! I am new to this community. Your comments are very >> welcome! >> > > Yeah, sorry to spoil the fun ;) Thanks for contributing, I hope the above > at least explains things. Testing is the key :) > > > Ciao, > Michael. >
_______________________________________________ Tinycc-devel mailing list [email protected] https://lists.nongnu.org/mailman/listinfo/tinycc-devel
