Re: [Tinycc-devel] problem on win64 with latest commit
On 01.03.2024 07:49, Herman ten Brugge via Tinycc-devel wrote: On 2/29/24 23:17, grischka wrote: On 29.02.2024 07:26, Herman ten Brugge via Tinycc-devel wrote: Setting CONFIG_RUNMEM_RO=0 looks incorrect to me because it sets write in executables. Apple has implemented W^X (Writes can not occur in executables) for security reasons. This may also be implemented in in future linux/bsd releases. Using CONFIG_RUNMEM_RO=1 may be the right thing to do in future, however there was a severe off-bounds problem with the un-mprotect call which I just fixed. Maybe that was the reason? This did not work. We still use 'PROT_READ | PROT_WRITE | PROT_EXEC'. Apple does not support that for security reasons. Last time you mentioned "Apple W^X", which according to https://developer.apple.com/documentation/apple-silicon/porting-just-in-time-compilers-to-apple-silicon would require mmap(), MAP_JIT, and some pthread_jit_write_protect_np() to work around. Since that is not what tcc has I was concluding that something else must be at work. Why do you want CONFIG_RUNMEM_RO=0? It was allways set to 1 before and that worked fine on all targets I can test (about 20). You changed it in "tccrun: resign from "advanced" system calls (memaligh/gettid)" on feb 25. Why? Some things have been simplified lately, the second argument to tcc_relocate() was removed, memalign was removed, etc. So in the course of going back to more simplicity, I did change that in order to see whether it still would work. Now it seems that it would work in most cases, except that it doesn't on "Apple Silicon M1 arm64". If that is what you're saying. Btw. I've seem some Apple arm64 related patches in https://github.com/frida/tinycc/commits/main/ such as https://github.com/frida/tinycc/commit/263232e8cf53991f195d7f7c028488cbd6f6b117 Anyway, I have no problem setting CONFIG_RUNMEM_RO=1 at all, we just need to be aware that it increases run-memory size by additional two pages. Since we also dropped memalign, the minimum run-memory size with CONFIG_RUNMEM_RO=1 now is 3 pages plus one for alignment, that is minimum 16 kB (4 x 4096 bytes). -- grischka Herman ___ Tinycc-devel mailing list Tinycc-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/tinycc-devel ___ Tinycc-devel mailing list Tinycc-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Re: [Tinycc-devel] problem on win64 with latest commit
On 2/29/24 23:17, grischka wrote: On 29.02.2024 07:26, Herman ten Brugge via Tinycc-devel wrote: Setting CONFIG_RUNMEM_RO=0 looks incorrect to me because it sets write in executables. Apple has implemented W^X (Writes can not occur in executables) for security reasons. This may also be implemented in in future linux/bsd releases. Using CONFIG_RUNMEM_RO=1 may be the right thing to do in future, however there was a severe off-bounds problem with the un-mprotect call which I just fixed. Maybe that was the reason? This did not work. We still use 'PROT_READ | PROT_WRITE | PROT_EXEC'. Apple does not support that for security reasons. Why do you want CONFIG_RUNMEM_RO=0? It was allways set to 1 before and that worked fine on all targets I can test (about 20). You changed it in "tccrun: resign from "advanced" system calls (memaligh/gettid)" on feb 25. Why? Herman ___ Tinycc-devel mailing list Tinycc-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Re: [Tinycc-devel] problem on win64 with latest commit
On 29.02.2024 07:26, Herman ten Brugge via Tinycc-devel wrote: Setting CONFIG_RUNMEM_RO=0 looks incorrect to me because it sets write in executables. Apple has implemented W^X (Writes can not occur in executables) for security reasons. This may also be implemented in in future linux/bsd releases. Using CONFIG_RUNMEM_RO=1 may be the right thing to do in future, however there was a severe off-bounds problem with the un-mprotect call which I just fixed. Maybe that was the reason? -- grischka Can I revert the change and set CONFIG_RUNMEM_RO=1 for all targets as before? Herman ___ Tinycc-devel mailing list Tinycc-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/tinycc-devel ___ Tinycc-devel mailing list Tinycc-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Re: [Tinycc-devel] problem on win64 with latest commit
I cannot speak for Griska or the reasons why that change was committed. But with respect to my mail from 2/25, once again, all I was doing was hinting that tcc could be configured to not call (the since reverted) unportable APIs. Anyway, from my point of view, a default of RO=1 is fine. But it should be #ifdef-protected so that tinycc/configure or win32/build-tcc.bat can change it. For instance: #ifndef CONFIG_RUNMEM_RO # define CONFIG_RUNMEM_RO 1 #endif But since I'm new here I'm not going to push such a commit without encouragement. Thanks - Eric On Wed, Feb 28, 2024 at 10:26 PM Herman ten Brugge wrote: > On 2/29/24 04:57, Eric Raible wrote: > > Huh? > > My email on 2/24 was definitely not intended to imply anything > about "old windows versions". I was simply trying to make a suggestion > about how to locally configure tcc to avoid a reported compiler error. > > In any case grischka responded 9 or so hours later to say > that the commit that caused that compiler error on windows > had been reverted. > > Griska reverted the memalign code but also made a change to > CONFIG_RUNMEM_RO=0. > I asume this was done because of your mail??? > > Setting CONFIG_RUNMEM_RO=0 looks incorrect to me because it sets write in > executables. > Apple has implemented W^X (Writes can not occur in executables) for > security reasons. > This may also be implemented in in future linux/bsd releases. > > Can I revert the change and set CONFIG_RUNMEM_RO=1 for all targets as > before? > > Herman > ___ Tinycc-devel mailing list Tinycc-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Re: [Tinycc-devel] problem on win64 with latest commit
On 2/29/24 04:57, Eric Raible wrote: Huh? My email on 2/24 was definitely not intended to imply anything about "old windows versions". I was simply trying to make a suggestion about how to locally configure tcc to avoid a reported compiler error. In any case grischka responded 9 or so hours later to say that the commit that caused that compiler error on windows had been reverted. Griska reverted the memalign code but also made a change to CONFIG_RUNMEM_RO=0. I asume this was done because of your mail??? Setting CONFIG_RUNMEM_RO=0 looks incorrect to me because it sets write in executables. Apple has implemented W^X (Writes can not occur in executables) for security reasons. This may also be implemented in in future linux/bsd releases. Can I revert the change and set CONFIG_RUNMEM_RO=1 for all targets as before? Herman___ Tinycc-devel mailing list Tinycc-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Re: [Tinycc-devel] problem on win64 with latest commit
Huh? My email on 2/24 was definitely not intended to imply anything about "old windows versions". I was simply trying to make a suggestion about how to locally configure tcc to avoid a reported compiler error. In any case grischka responded 9 or so hours later to say that the commit that caused that compiler error on windows had been reverted. - Eric On Tue, Feb 27, 2024 at 10:51 PM Herman ten Brugge via Tinycc-devel < tinycc-devel@nongnu.org> wrote: > On 2/27/24 19:51, grischka wrote: > > On 26.02.2024 19:55, Herman ten Brugge via Tinycc-devel wrote: > >> I commited a change to lib/runmain.c to silence a warning from gcc. > >> After that testcase 112 did not work any more on 64 bits windows > >> (tested with wine). > >> > >> I can fix this with fix1 (see below) or fix2 (see below). > >> I do not know enough about windows to make the correct fix. > >> Both fix1/fix2 look not correct to me. > > > > Hi Herman, > > > > well, first we'd need to know what problem we want to fix. Maybe > > the problem is not in tinycc at all. > > > > Also, the obvious to avoid that gcc warning IMO would be using: > > > > __attribute__((noreturn)) void __rt_exit(rt_frame *, int); > > > > Also, while at it: > > > > # ifdef _WIN32 > > # define CONFIG_RUNMEM_RO 0 > > # else > > # define CONFIG_RUNMEM_RO 1 > > # endif > > > > would suggest a special case needed for windows. However that is not > > the case. Whereas according to your comment, the exception is APPLE > > actually. So we'd better say #ifdef __APPLE__ or just use '1' for all > > and leave a comment that it's needed for APPLE. > > > > Although tests on github actions did not show such problem, at least > > not on macos-11. Note that CONFIG_RUNMEM_RO was first introduced with > > .rodata sections in commit 72f1dea5372ed551d203311e4f2ab769a54de9bd > > from 2021-02, and it seems that tcc already did work on MacOS even > > before that commit. If those previous commits still work then the > > problem now might be elsewhere. > > > > Anyway, we cannot disable the runtime-function-unwind table because > > without it longjmp() would crash on (a real) _WIN64. > > > > Also obviously, if __rt_exit doesn't return, then any code after it > > cannot have any effect: > > > >> __rt_exit(, code); > >> +abort (); // avoid noreturn warning > > > > I'd assume something weird going on with set/longjmp of WINE and/or > > the mingw-gcc that you use. In any case it is important that > > setjmp/longjmp/jmp_buf implementations do match each other. > > > > Which is why I made it so that tcc_setjmp records the longjmp > > function pointer too because otherwise it could end up using the > > longjmp from libtcc.dll/so and the setjmp from the application > > that links to it, with possibly different concepts each. (Not > > sure if/where it really can happen though). > > > There is probably a bug in the wine stack unwinding code. > The __attribute__((noreturn)) looks like the correct fix. I will apply it. > > I made the change to CONFIG_RUNMEM_RO because it was set to 1 > before and that worked on all targets. I saw a mail from Eric Raible > on 25 Feb 2024 that it should be set to 0 for old window versions. > This is why I made the change to only effect windows. > On most targets setting it to 0 or 1 did work. (Not tested on all) > > I use an apple machine from the gcc compile farm. > Darwin cfarm104.cfarm.net 21.6.0 Darwin Kernel Version 21.6.0: Mon Aug > 22 20:20:05 PDT 2022; root:xnu-8020.140.49~2/RELEASE_ARM64_T8101 arm64 > I did not check why it failed but with CONFIG_RUNMEM_RO=0 it reported: > mprotect failed (did you mean to configure --with-selinux?) > Using --with-selinux on apple works but looked strange. > > I also noticed that maybe --with-selinux is not needed any more > with the recent changes. I removed it on all my test targets. > > Herman > > > ___ > Tinycc-devel mailing list > Tinycc-devel@nongnu.org > https://lists.nongnu.org/mailman/listinfo/tinycc-devel > ___ Tinycc-devel mailing list Tinycc-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Re: [Tinycc-devel] problem on win64 with latest commit
On 2/27/24 19:51, grischka wrote: On 26.02.2024 19:55, Herman ten Brugge via Tinycc-devel wrote: I commited a change to lib/runmain.c to silence a warning from gcc. After that testcase 112 did not work any more on 64 bits windows (tested with wine). I can fix this with fix1 (see below) or fix2 (see below). I do not know enough about windows to make the correct fix. Both fix1/fix2 look not correct to me. Hi Herman, well, first we'd need to know what problem we want to fix. Maybe the problem is not in tinycc at all. Also, the obvious to avoid that gcc warning IMO would be using: __attribute__((noreturn)) void __rt_exit(rt_frame *, int); Also, while at it: # ifdef _WIN32 # define CONFIG_RUNMEM_RO 0 # else # define CONFIG_RUNMEM_RO 1 # endif would suggest a special case needed for windows. However that is not the case. Whereas according to your comment, the exception is APPLE actually. So we'd better say #ifdef __APPLE__ or just use '1' for all and leave a comment that it's needed for APPLE. Although tests on github actions did not show such problem, at least not on macos-11. Note that CONFIG_RUNMEM_RO was first introduced with .rodata sections in commit 72f1dea5372ed551d203311e4f2ab769a54de9bd from 2021-02, and it seems that tcc already did work on MacOS even before that commit. If those previous commits still work then the problem now might be elsewhere. Anyway, we cannot disable the runtime-function-unwind table because without it longjmp() would crash on (a real) _WIN64. Also obviously, if __rt_exit doesn't return, then any code after it cannot have any effect: __rt_exit(, code); + abort (); // avoid noreturn warning I'd assume something weird going on with set/longjmp of WINE and/or the mingw-gcc that you use. In any case it is important that setjmp/longjmp/jmp_buf implementations do match each other. Which is why I made it so that tcc_setjmp records the longjmp function pointer too because otherwise it could end up using the longjmp from libtcc.dll/so and the setjmp from the application that links to it, with possibly different concepts each. (Not sure if/where it really can happen though). There is probably a bug in the wine stack unwinding code. The __attribute__((noreturn)) looks like the correct fix. I will apply it. I made the change to CONFIG_RUNMEM_RO because it was set to 1 before and that worked on all targets. I saw a mail from Eric Raible on 25 Feb 2024 that it should be set to 0 for old window versions. This is why I made the change to only effect windows. On most targets setting it to 0 or 1 did work. (Not tested on all) I use an apple machine from the gcc compile farm. Darwin cfarm104.cfarm.net 21.6.0 Darwin Kernel Version 21.6.0: Mon Aug 22 20:20:05 PDT 2022; root:xnu-8020.140.49~2/RELEASE_ARM64_T8101 arm64 I did not check why it failed but with CONFIG_RUNMEM_RO=0 it reported: mprotect failed (did you mean to configure --with-selinux?) Using --with-selinux on apple works but looked strange. I also noticed that maybe --with-selinux is not needed any more with the recent changes. I removed it on all my test targets. Herman ___ Tinycc-devel mailing list Tinycc-devel@nongnu.org https://lists.nongnu.org/mailman/listinfo/tinycc-devel
Re: [Tinycc-devel] problem on win64 with latest commit
On 26.02.2024 19:55, Herman ten Brugge via Tinycc-devel wrote:
I commited a change to lib/runmain.c to silence a warning from gcc.
After that testcase 112 did not work any more on 64 bits windows (tested with
wine).
I can fix this with fix1 (see below) or fix2 (see below).
I do not know enough about windows to make the correct fix.
Both fix1/fix2 look not correct to me.
Hi Herman,
well, first we'd need to know what problem we want to fix. Maybe
the problem is not in tinycc at all.
Also, the obvious to avoid that gcc warning IMO would be using:
__attribute__((noreturn)) void __rt_exit(rt_frame *, int);
Also, while at it:
# ifdef _WIN32
# define CONFIG_RUNMEM_RO 0
# else
# define CONFIG_RUNMEM_RO 1
# endif
would suggest a special case needed for windows. However that is not
the case. Whereas according to your comment, the exception is APPLE
actually. So we'd better say #ifdef __APPLE__ or just use '1' for all
and leave a comment that it's needed for APPLE.
Although tests on github actions did not show such problem, at least
not on macos-11. Note that CONFIG_RUNMEM_RO was first introduced with
.rodata sections in commit 72f1dea5372ed551d203311e4f2ab769a54de9bd
from 2021-02, and it seems that tcc already did work on MacOS even
before that commit. If those previous commits still work then the
problem now might be elsewhere.
Anyway, we cannot disable the runtime-function-unwind table because
without it longjmp() would crash on (a real) _WIN64.
Also obviously, if __rt_exit doesn't return, then any code after it
cannot have any effect:
__rt_exit(, code);
+abort (); // avoid noreturn warning
I'd assume something weird going on with set/longjmp of WINE and/or
the mingw-gcc that you use. In any case it is important that
setjmp/longjmp/jmp_buf implementations do match each other.
Which is why I made it so that tcc_setjmp records the longjmp
function pointer too because otherwise it could end up using the
longjmp from libtcc.dll/so and the setjmp from the application
that links to it, with possibly different concepts each. (Not
sure if/where it really can happen though).
-- grischka
Herman
fix1:
diff --git a/lib/runmain.c b/lib/runmain.c
index 1cbf6dd..307bf45 100644
--- a/lib/runmain.c
+++ b/lib/runmain.c
@@ -60,6 +60,7 @@ typedef struct rt_frame {
} rt_frame;
void __rt_exit(rt_frame *, int);
+void abort(void);
void exit(int code)
{
@@ -69,7 +70,7 @@ void exit(int code)
f.fp = 0;
f.ip = exit;
__rt_exit(, code);
-for (;;); // avoid noreturn warning
+abort (); // avoid noreturn warning
}
#ifndef _WIN32
fix2:
diff --git a/tccrun.c b/tccrun.c
index b27ccc5..5ea271e 100644
--- a/tccrun.c
+++ b/tccrun.c
@@ -456,7 +456,7 @@ static int protect_pages(void *ptr, unsigned long length,
int mode)
static void *win64_add_function_table(TCCState *s1)
{
void *p = NULL;
-if (s1->uw_pdata) {
+if (0&>uw_pdata) {
p = (void*)s1->uw_pdata->sh_addr;
RtlAddFunctionTable(
(RUNTIME_FUNCTION*)p,
___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel
