Windows support: strategy question (linux emulation or "native")

Not sure if this is the right repository to place this issue,
since it also affects ccir, but i feel it might belong a bit more here.
Is a windows implementation planned?
I see two very different ways of implementing it:

emulating the required linux APIs ("mapping" to the windows equivalents
at runtime or at a later stage of the project during compilation)
implementing it natively (as the linux implementation, proxying WINAPI calls,
this seems to require quite much work?) <---

Which of those (or maybe an other even better idea?) would you consider the
favorable solution?
Other todos:

[ ] Get rid of several win stubs (by not requiring them on win)
[ ] get rid of workaround, temporary solutions, ...
[ ] x86

@steffengy

Is a windows implementation planned?

Yes, but I hope someone knowing Windows will take care of it.

emulating the required linux APIs ("mapping" to the windows equivalents
at runtime or at a later stage of the project during compilation)

I think emulating one OS in another, while possible, is too ambitious. virtual uses direct syscalls wherever possible, because I have no intents to rewrite parts of libc or the Linux kernel from scratch ;-)

implementing it natively (as the linux implementation, proxying WINAPI calls,
this seems to require quite much work?)

IIUC, this is probably the better way.

Couple of notes.
There's a set of platform/architecture neutral VM opcodes, their respective implementations and there's a platform/achitecture specific set. The later set can be tailored using build tags or using the foo_os_arch.go naming scheme. The later option is already in use where amd64 and 386 architectures differ on Linux, so I guess it's easiest to stick with that.
Assuming sqlite is the motivation, I suggest to start the approach like this, because ccir consults virtual when compiling C to recognize functions implemented by the VM itself, aka "builtins":

Install gcc on a Windows box.
Install ccir, go to its libc directory, enter go generate. See what happens and provide feedback so I can make any necessary changes.
Expand sqlite-amalgamation-3180000.zip to $GOPATH/src/sqlite.org/sqlite-amalgamation-3180000 so ccir tests can find the sources.
Invoke go test. That will fail, but the failures will tell exactly which "syscalls" sqlite needs on Windows.
Implement them all in the way how it's done on Windows. (I have little idea how.)
Once ccir and virtual tests pass on Windows, enter the sqlite directory, try issuing go generate and, if that works, go test.

Easy, isn't it? ;-)

Thanks for the valuable input!

Assuming sqlite is the motivation

Assumed correctly ;)

Install ccir, go to its libc directory, enter go generate. See what happens and provide feedback so I can make any necessary changes.

..\..\virtual\stdio.go:17:2: no buildable Go source files in gopath\src\github.com\cznic\ccir\libc\errno
..\..\virtual\pthread.go:12:2: no buildable Go source files in gopath\src\github.com\cznic\ccir\libc\pthread
..\..\virtual\stdio.go:18:2: no buildable Go source files in gopath\src\github.com\cznic\ccir\libc\stdio
..\..\virtual\unistd.go:14:2: no buildable Go source files in gopath\src\github.com\cznic\ccir\libc\unistd

Which is so far expected. Should virtual really depend on pthreads for windows?
(That seems quite linux-specific, even though there are pthread-shims for windows)

That's a rather unfortunate dependency chain we've got here. generator.go in ccir/libc must import ccir, which imports virtual which imports things under ccir/libc which exactly generator.go should generate ;-)
The proper solution is not clear to me immediately, IIRC, the virtual.IsBuiltin function is the only thing ccir, modulo its tests, needs from virtual.
Let's do the "improper" way: Copy the ccir/libc/errno/errno_linux_yyy.go file to ccir/libc/errno/errno_windows_yyy.go, where yyy is your GOARCH. That will make the constants in that package probably wrong, but I believe it should not matter for now as generator.go generates it's stuff using gcc's headers.
Of course, you need to repeat the step above for any and all other directories missing Windows specific files. Commit to some temp branch before running go generate again because those files will get overwritten once generator.go builds and attempts to do its job. After that you should have the correct values there. Or no values[0] where the source thing does not exist on Windows (like pthread.h). This situation will indicate where a file in virtual will later[1] need to be specialized (like virtual/pthread.go -> virtual/pthread_linux.go, for example).
[0]: Such instances would be probable removed completely later on - once they get no more imported on Windows.
[1]: Maybe this will be necessary before ccir/libc/generator can build, I'm not sure, but it seems you're soon going to know ;-)

@cznic
I created the necessary _windows_amd64 files, now go generate results in:
..\..\virtual\cpu.go:1747: c.lstat64 undefined (type *cpu has no field or method lstat64)
..\..\virtual\cpu.go:1749: c.stat64 undefined (type *cpu has no field or method stat64)
..\..\virtual\cpu.go:1755: c.open64 undefined (type *cpu has no field or method open64)
..\..\virtual\cpu.go:1757: c.fcntl undefined (type *cpu has no field or method fcntl)
..\..\virtual\cpu.go:1759: c.fstat64 undefined (type *cpu has no field or method fstat64)
..\..\virtual\sys_mman.go:28: undefined: syscall.SYS_MMAP
..\..\virtual\sys_mman.go:28: not enough arguments in call to syscall.Syscall6
        have (<T>, uintptr, uintptr, uintptr, uintptr, uintptr, uintptr)
        want (uintptr, uintptr, uintptr, uintptr, uintptr, uintptr, uintptr, uintptr)
..\..\virtual\sys_mman.go:39: undefined: syscall.SYS_MUNMAP
..\..\virtual\sys_mman.go:39: not enough arguments in call to syscall.Syscall
        have (<T>, uintptr, uintptr, number)
        want (uintptr, uintptr, uintptr, uintptr, uintptr)
..\..\virtual\sys_time.go:24: undefined: syscall.SYS_GETTIMEOFDAY
..\..\virtual\sys_time.go:24: too many errors

I have a feeling that quite much more in virtual currently depends on linux.

I have a feeling that quite much more in virtual currently depends on linux.

You're right and it's my fault. IMD, free time is most precious and I thought about porting, but that's about it, sorry.
But please don't give up! We can make it, I'm sure ;-)
I'll create ccir and virtual branches for the Windows port, send you a collaborator's invite and will try to fix the trouble in the parts I am able to do so. The above quoted errors seem to be well in that category. IOW, I'll prepare any stubs needed for you to fill in the implementation any time later.
After you accept the invitations, if you decide to do so, please push to the respective branches the non-building versions you have now and I'll try to do what I can. Please be patient, I cannot really test my part, I don't have any Windows box at hand now, not even a virtualized one, so it will probably take more than one round to get it building at least.

@steffengy Branches are ready, invitations sent.

@cznic Thanks for getting me started, I committed my local CCIR changes (which consist of only copying the _linux_amd64.go files to _windows_amd64 as discussed above), for virtual there aren't any changes yet.

Thanks! I'm on it.

@steffengy
jnml@4670:~/src/github.com/cznic/ccir/libc$ GOOS=windows go build generator.go
jnml@4670:~/src/github.com/cznic/ccir/libc$ file generator.exe
generator.exe: PE32+ executable (console) x86-64 (stripped to external PDB), for MS Windows
jnml@4670:~/src/github.com/cznic/ccir/libc$

Seems okay here, but I cannot of course execute it.

If I understand correctly VARARGS aren't supported by cznic/cc yet?

No, varargs should work just fine.
jnml@4670:~/src/github.com/cznic/cc$ git branch
  issue51
  issue57
  issue59
  issue60
  issue62
  issue67
  issue67-debug
  issue67-wips
  issue91
* master
jnml@4670:~/src/github.com/cznic/cc$ git log -1
commit d5c3a9df03dc0d4053f7d1ba6f4e4622d6d645dc
Author: Jan Mercl<EMAIL_ADDRESS>Date:   Wed Apr 12 01:05:53 2017 +0200

    Fix anonymous struct/union members flattening.
jnml@4670:~/src/github.com/cznic/cc$ grep -ni varargs *
cc.go:419:// See https://gcc.gnu.org/onlinedocs/gccint/Varargs.html
grep: testdata: je adresářem
jnml@4670:~/src/github.com/cznic/cc$

The above and what you sent seems to indicate the error message "VARARGS not implemented for this compiler" probably comes from an #error directive. Why? Good question I don't have an answer for ATM.
Can you please take a look at m:\mingw\x86_64-w64-mingw32\include\vadefs.h:35:2 and around to look what the surround (guess) #if or #ifdef condition is? I will meanwhile try to google the file as well. Thank you.

@cznic Already updated the message, seems like you were to fast :)

Looking here I think we probaly nedd #define _MSC_VER somenumber on Windows platform. This is due to a hack where I remove any GNUC|GCC #define from the set of predefined macros. It's necessary because otherwise GNU libc wants very GCC specific things.

Looking here I think we probaly need #define _MSC_VER somenumber on Windows platform.

Hopefully that won't pull in anything too MSVC C++ specific, but I also
do not see what else would help here.

 23:11:45.068749 generator.go:826: m:\mingw\x86_64-w64-mingw32\include\crtdefs.h:35:44: unexpected identifier size_t, expected one of ['(', ')', ',', ':', ';', '=', '[', '{', _Bool, _Complex, _Noreturn, _Static_assert, asm, auto, char, const, double, enum, extern, float, inline, int, long, register, restrict, short, signed, static, struct, typedef, typedefname, typeof, union, unsigned, void, volatile]
Seems like cznic/cc isn't able to parse crtdefs.h:35 which is:
__MINGW_EXTENSION typedef unsigned __int64 size_t;

That part is a bug (or not implemented), I guess?

I hoped for GCC on Windows to also use its own (GNU libc) headers, but it doesn't seem to be the case. It took a bit of tweaking to make it all work with the GNU libc headers...

That part is a bug (or not implemented), I guess?

I think this can be resolved by adding
#define __MINGW_EXTENSION
to builtin.h (in ccir/libc).

I added the following as a test to ccir/libc/builtin.h:
#define _MSC_VER
#define __MINGW_EXTENSION

This didn't change any of the error messages, which is strange, since atleast the VARARGS error should surely disappear. Do I need to follow some additional steps that these changes are applied?

This didn't change any of the error messages, which is strange, since atleast the VARARGS error should surely disappear.

Very strange, I must be missing something.
Not really related to the above, but maybe __int64 is not defined to be a type and it neeeds to be added to builtin.h in Windows?

#define _MSC_VER is possibly not enough, try giving it a non zero value, like in #define _MSC_VER 1, for example.

Also tried that, adding some random stuff like abc which should be syntactically invalid on it's own
also doesn't result in any change of the errors (which it surely should).
So I guess it doesn't actually look at this file?

I'll try to add a cpp hook to ccir/libc/generator.go, it may help in debugging the issue.

Option added, please pull and try go run generator.go -cpp instead of go generate and redirect stderr output to some log file to inspect. You can also attach it here at issue comments for me to have a look, I think it's possible.

Done:
# 22 "predefined.h"
typedef char *__builtin_va_list;
# 548 "m:\\mingw\\x86_64-w64-mingw32\\include\\_mingw.h"
const char *__mingw_get_crt_info (void);
# 35 "m:\\mingw\\x86_64-w64-mingw32\\include\\crtdefs.h"
typedef unsigned __int64 size_t;
00:00:39.304571 generator.go:847: mingw\x86_64-w64-mingw32\include\crtdefs.h:35:44: unexpected identifier size_t, expected one of ['(', ')', ',', ':', ';', '=', '[', '{', _Bool, _Complex, _Noreturn, _Static_assert, asm, auto, char, const, double, enum, extern, float, inline, int, long, register, restrict, short, signed, static, struct, typedef, typedefname, typeof, union, unsigned, void, volatile]
mingw\x86_64-w64-mingw32\include\vadefs.h:35:2: error: "VARARGS not implemented for this compiler"
exit status 1

# 35 "m:\\mingw\\x86_64-w64-mingw32\\include\\crtdefs.h"
typedef unsigned __int64 size_t;
00:00:39.304571 generator.go:847: mingw\x86_64-w64-mingw32\include\crtdefs.h:35:44: unexpected identifier size_t, expected one of ['(', ')', ',', ':', ';', '=', '[', '{', _Bool, _Complex, _Noreturn, _Static_assert, asm, auto, char, const, double, enum, extern, float, inline, int, long, register, restrict, short, signed, static, struct, typedef, typedefname, typeof, union, unsigned, void, volatile]

The crtdefs.h:35:44 seems to have only 32 characters, why the error column is 44? Do you have any idea?
Anyway, please look into the log, if __int64 is typedefed or #defined any place before usage here. Thanks.

EDIT: seems like builtin.h is indeed somehow not applied.

Have no idea why, it's imported quite directly.

The crtdefs.h:35:44 seems to have only 32 characters, why the error column is 44?

"__MINGW_EXTENSION typedef unsigned __int64 size_t;".length == 50

Anyway, please look into the log, if __int64 is typedefed or #defined any place before usage here.

With the following in predefined.h
#define _MSC_VER 1
#define __MINGW_EXTENSION 1

the (entire) log is:
# 25 "predefined.h"
typedef char *__builtin_va_list;
# 33 "m:\\mingw\\x86_64-w64-mingw32\\include\\vadefs.h"
typedef char * va_list;
# 548 "m:\\mingw\\x86_64-w64-mingw32\\include\\_mingw.h"
const char *__mingw_get_crt_info (void);
# 35 "m:\\mingw\\x86_64-w64-mingw32\\include\\crtdefs.h"
typedef unsigned __int64 size_t;
00:12:16.470538 generator.go:847: m:\mingw\x86_64-w64-mingw32\include\crtdefs.h:35:44: unexpected identifier size_t, expected one of ['(', ')', ',', ':', ';', '=', '[', '{', _Bool, _Complex, _Noreturn, _Static_assert, asm, auto, char, const, double, enum, extern, float, inline, int, long, register, restrict, short, signed, static, struct, typedef, typedefname, typeof, union, unsigned, void, volatile]
exit status 1

Adding e.g. #define __int64 int [which isn't quite right but fine to track this compiler issue here]
to also predefined.h (definitely also not working in builtin.h)
seems to do it and gives me another error (the next one):
ctype_windows_amd64.h:10:23: unexpected identifier __ptlocinfo, expected one of ['(', ')', ',', ':', ';', '=', '[', '{', _Bool, _Complex, _Noreturn, _Static_assert, asm, auto, char, const, double, enum, extern, float, inline, int, long, register, restrict, short, signed, static, struct, typedef, typedefname, typeof, union, unsigned, void, volatile]

My comment about <builtin.h> was misleading, sorry, that's used in a later stage.
I think your experiments are correct, we have to, guarded by some #ifdefs, add some #defines to predefine.h.
Sorry, it's few weeks I've been into these places and got meanwhile focused elsewhere...

ctype_windows_amd64.h:10:23: unexpected identifier __ptlocinfo, expected one of ['(', ')', ',', ':', ';', '=', '[', '{', _Bool, _Complex, _Noreturn, _Static_assert, asm, auto, char, const, double, enum, extern, float, inline, int, long, register, restrict, short, signed, static, struct, typedef, typedefname, typeof, union, unsigned, void, volatile]

Such implementation defined things, note the two leading underscores, are why we are slurping the predefined macros from cpp -dM. What does that, if I recall the command correctly, shows?

BTW, have I told you how much I dislike C after doing this project? ;-)

Haha you probably now several more edge cases than before :)
cpp -dM doesn't appear to do anything.
Maybe __ptlocinfo comes from mingw-w64-headers/crt/ctype.h ?

Haha you probably now know several more edge cases than before :)

Always look on the bright side ;-)

cpp -dM doesn't appear to do anything.

Yeah, on Linux it should be $ cpp -dM /dev/null, please try (guessed) c:\> cpp -dM NUL or something like that... We rely on the system CC predefines a lot.

Maybe __ptlocinfo comes from mingw-w64-headers/crt/ctype.h ?

If only I had any idea what it stands for. Some 'pointer to location info'?

Or 'thread local info' maybe?

Since it's quite big not directly here (cpp -dM NUL works):
https://gist.github.com/steffengy/690934e545549c280efa6731ac4550eb
pthreadlocinfo "points" to threadlocaleinfostruct.

Since it's quite big not directly here (cpp -dM NUL works):
https://gist.github.com/steffengy/690934e545549c280efa6731ac4550eb

Thanks. The output of cpp -dM seems okay. That's all I know.
We're now in the Windows/C land and there's not much left what I can help you with. Not that I don't want to, I just don't know enough about Windows (and the GCC implementation on that platform).
Please try to investigate further. Don't hesitate to ask for any help I can provide you with. (That means mostly the Go side, unfortunately.) We can always ask other/more experts if necessary. Last, but not least, it's getting a bit late at night here and I should probably get some sleep.
My wish for this weekend is to try and start coding ccgo. FYI: The ccir/libc *.h and *.go stuff will be used by it as well, meaning this work of your's is not going to be wasted.
Also, please add yourself to the CONTRIBUTORS and/or AUTHORS files in both ccir and virtual so we do not forget to do that later before merging the issue1 branches to master.

Yeah I'll try to look into that stuff once I got proper defines for __int64
and will update here once there are some exciting news.
The windows land itself is often quite annoying, MSVC even more
but MINGW is IMHO not better buf (except for interoperability) worse.

Also signing out for today.
Regarding  unexpected identifier __ptlocinfo for extern pthreadlocinfo __ptlocinfo;
seems like that it's actually because of pthreadlocalinfo not being recognized.

Using #define pthreadlocinfo struct threadlocaleinfostruct  "fixes" the compilation,
while using the equivalent typedef doesn't work.
It actually should pick this definition up from the explicitily included header
(did it in prebuild.h but it doesn't appear so)

From my point this seems like a bug, where typedefs are not semantically recognized for such
cases?

From my point this seems like a bug, where typedefs are not semantically recognized for such
cases?

Of course it can be a bug. If you can create a simple reproduction case, which some other C compiler accepts, please fill a cc issue, thank you.

@cznic
LIBC builds now for mingw64 :)
Happy for feedback, especially regarding the workaround + adding of types here: https://github.com/cznic/ccir/commit/c77c34348497f3d1b39778e50a099d6b66eaf18e#diff-c0cbe5fc2255f37eda2c46a58fd84141R930

Assuming "LIBC builds" means that running go generate in ccir/libc works without errors, I'm not sure from where or at which step the error in the header file is reported. Is that when go test is invoked in ccir? Please clarify.
Also, please keep pushing your intermediate local changes of the issue1 branches so I can early test for potential breakages of the Linux version. Don't worry about size of the commits or proper commit messages if you think the  later merge to master branch will be squashed.
In any case, it seems your porting effort is making great progress already!
I will get to look cznic/cc#94 in a couple of hours, I hope.

Assuming "LIBC builds" means that running go generate in ccir/libc works without errors, I'm not sure from where or at which step the error in the header file is reported. Is that when go test is invoked in ccir? Please clarify.

Sorry, yeah that happens when I run go test when for example a GCC-torture test relies on string.h and stdlib.h, which both contain a definition of pthreadlocinfo, which fails because of cznic/cc#94.
I'm currently also trying to find out what's going on there, but still having a hard time
finding the relevant place in the parser.

Wow! Do some tests already pass? You're light years ahead compared to what I thought is possible only yesterday ;-)

--- FAIL: TestTCC (0.65s)
        all_test.go:359: ..\cc\testdata\tcc-0.9.26\tests\tests2\22_floating_point.c: FAIL
                cc.Parse: gopath\src\github.com\cznic\ccir\libc\math_windows_amd64.h:23:100: unexpected ',', expected declarator or optional init declarator list or one of ['(', '*', ';', identifier]
        all_test.go:368: 47/55 ok
        all_test.go:370: failures: 8
--- FAIL: TestGCCExec (10.51s)
        all_test.go:359: ..\cc\testdata\gcc-6.3.0\gcc\testsuite\gcc.c-torture\execute\20000910-2.c: FAIL
                cc.Parse: gopath\src\github.com\cznic\ccir\libc\string_windows_amd64.h:23:100: unexpected ',', expected declarator or optional init declarator list or one of ['(', '*', ';', identifier]
        all_test.go:368: 1222/1250 ok
        all_test.go:370: failures: 28
--- FAIL: TestC4 (0.02s)
        all_test.go:792: D:\projects\Go\gopath\src\github.com\cznic\ccir\libc\stdlib_windows_amd64.h:23:100: unexpected ',', expected declarator or optional init declarator list or one of ['(', '*', ';', identifier]
--- FAIL: TestSqlite (0.06s)
        all_test.go:792: gopath\src\github.com\cznic\ccir\libc\stdlib_windows_amd64.h:23:100: unexpected ',', expected declarator or optional init declarator list or one of ['(', '*', ';', identifier]
FAIL
exit status 1
FAIL    github.com/cznic/ccir   11.312s

I guess that means some already pass ;-)

    ir.LinkMain: ..\cc\testdata\tcc-0.9.26\tests\tests2\40_stdio.c:5:14: undefined fopen
                # ccir.New
                # [0]: *ir.FunctionDefinition {gopath\src\github.com\cznic\ccir\libc\builtin.h:38:15 __builtin_strlen func(*int8)uint64 ExternalLinkage } [__s]
                0x00000         panic                           ; gopath\src\github.com\cznic\ccir\libc\builtin.h:38:15
                # [1]: *ir.FunctionDefinition {gopath\src\github.com\cznic\ccir\libc\builtin.h:39:17 __builtin_bswap64 func(uint64)uint64 ExternalLinkage } [x]
                0x00000         panic                           ; gopath\src\github.com\cznic\ccir\libc\builtin.h:39:17

What does output like that actually mean?
Seems like builtin.h comes into play now, but since stdio.h for windows contains
those definitions I guess this is "one stage further" now?

Blind shot: https://github.com/cznic/virtual/commit/951399f7ea15f610de34277e3f938fd04d5aa676 Please try, thank you.
   # ccir.New
                # [0]: *ir.FunctionDefinition {gopath\src\github.com\cznic\ccir\libc\builtin.h:38:15 __builtin_strlen func(*int8)uint64 ExternalLinkage } [__s]
                0x00000         panic                           ; gopath\src\github.com\cznic\ccir\libc\builtin.h:38:15
                # [1]: *ir.FunctionDefinition {gopath\src\github.com\cznic\ccir\libc\builtin.h:39:17 __builtin_bswap64 func(uint64)uint64 ExternalLinkage } [x]
                0x00000         panic                           ; gopath\src\github.com\cznic\ccir\libc\builtin.h:39:17

This lists/dumps the objects ir.LinkMain was passed to aid in debugging linker failure.

@cznic It may've been blind but spot on :)
Now I atleast know that for the future errors of this kind,
I'll have to take a look at virtual.

EDIT: likely fopen should point to __builtin_fopen (which is in builtin.h)

That could be the proper solution. It depends on how GCC on Windows solves the 32/64 bit file sizes schizma of many libc functions. This solves it for Linux.

Now I atleast know that for the future errors of this kind, I'll have to take a look at virtual.

Yeah, you're now entering the real land-of-fun, aka the virtual builtins B-)

FYI: Builtins are implemented as "abnormal" functions having a single panic instruction. That's a pattern recognized in virtual.Load and replaced by a call to the corresponding Go builtin. Like fopen64 you've just met.

GCC & TCC test suites now pass on windows. Now the real fun begins.

GCC & TCC test suites now pass on windows. Now the real fun begins.

Wonderful. Just fantastic!

BTW currently working on implementing open64 for windows
and wondering what the reason to use syscall.syscall(SYS_OPEN over syscall.Open for linux
is? (For windows that might lead to saving several lines of syscall-boilerplate)

BTW currently working on implementing open64 for windows
and wondering what the reason to use syscall.syscall(SYS_OPEN over syscall.Open for linux
is? (For windows that might lead to saving several lines of syscall-boilerplate)

Simply never considered. Direct mapping of libc syscallls to real syscalls was the first attempt tried - and kept if it happened to just work.

Just wondering if there's a performance implication (not sure how aggressively GO inlines).
I'll probably do a short benchmark and then use whatever seems the best solution for each _individual  case.

NOTE: Doesn't do inlining for syscall.Open after looking at the assembly. There's also additional overhead for checks the GO-code contains (which we might not want) so that route of direct mapping seems to make  sense here too.

Don't worry about performance too much - yet. The VM is already ~100 times slower than native code. Improving a particular libc function implemented in Go can thus have a rather limited impact on the overall speed, unless executed extremely often.
Even more importantly, I still hope to be able to soon-ish-ly translate the IR to Go. That's the strategic performance-wise optimization target. The ccgo project will use the ccir/libc generated .h/.go files and it will re-use the virtual/name_os_arch.go libc implementing stuff. Initially perhaps just by copy-pasting parts of it. Later some common crt (C run time) may become reasonable.
There's an intermediate step possible which I'm still contemplating: Instead of translating IR to Go, it's possible also to "unroll" the VM opcodes to the semantically equal Go code. According to some previous experiments, that would speedup most things by at least ~50%. But speedups are sometimes per se a bit mysterious mater ;-)

NOTE: Doesn't do inlining for syscall.Open after looking at the assembly. There's also additional overhead for checks the GO-code contains (which we might not want) so that route of direct mapping seems to make sense here too.

Be lazy as much as you can. There's still some work to do ahead where you'll probably don't have the luxury option of just calling the kernel.
OTOH, you're now about a dozen of "syscalls" away from having your Windows port complete. You are incredible. Especially when I observe you can easily understanding the spaghetti code of mine you have to deal with :open_mouth:
Kudos.

Still having some issues with builtin.h:
__builtin_offsetof is required by windows.h but not found during go generate.
(I added windows.h to generator.go)
If I add #define __builtin_offsetof(st, m) ((__SIZE_TYPE__)(&((st *)0)->m)) from builtin.h
to predefined.h I get some errors because something seems to pull in the definitions of ___header etc.
into stddef_windows_amd64.h, which then errors with cannot redefine macro using a replacement list of different length.
IMO This should be picked up from builtin.h, how does that work on linux?

On Linux: __builtin_offsetof is defined in builtin.h, which which does #include <predefined.h>. Ie. defining it again in predefined is not the correct place. predefined.h should define only what cpp -dM shows and what was slurped when doing go generate.
Please try to run the test like, let's say the problem shows in TestGCCExec test named foo.c
$ go test -v -cpp -run GCC -re foo.c

and redirect stderr to a log. This way you can inspect exactly what the compiler sees after preprocessing foo.c and track exactly the source of every definition/redefinition. HTH

How should I go about that since go generate for windows.h already fails with:
m:\mingw\x86_64-w64-mingw32\include\winnt.h:8359:100: unexpected typedefname NT_TIB, expected optional argument expression list or one of [&&, '!', '&', '(', ')', '*', '+', '-', '~', ++, --, _Alignof, character constant, floating-point constant, identifier, integer constant, long character constant, long string constant, sizeof, string literal]

648: #define FIELD_OFFSET(Type, Field) ((LONG) __builtin_offsetof(Type, Field))
8359:  FORCEINLINE struct _TEB *NtCurrentTeb(VOID) { return (struct _TEB *)__readgsqword(FIELD_OFFSET(NT_TIB,Self)); }

The code compiled during go generate AFAIK already includes a `#include <builtin.h>' but as
we previously noted, the compiler doesn't seem to be very keen to use that here.

Ah, I thought go generate worked already. You're trying to remove some temporary solutions, I guess. There you can try go run generate.go -cpp and hopefully you will get the preprocessor output. But you seem to be doing that already.
m:\mingw\x86_64-w64-mingw32\include\winnt.h:8359:100: unexpected typedefname NT_TIB, expected optional argument expression list or one of [&&, '!', '&', '(', ')', '*', '+', '-', '~', ++, --, _Alignof, character constant, floating-point constant, identifier, integer constant, long character constant, long string constant, sizeof, string literal]

648: #define FIELD_OFFSET(Type, Field) ((LONG) __builtin_offsetof(Type, Field))
8359:  FORCEINLINE struct _TEB *NtCurrentTeb(VOID) { return (struct _TEB *)__readgsqword(FIELD_OFFSET(NT_TIB,Self)); }

In the above, is FORCELINE defined to be something? I would guess it should defined as nothing, i.e only #define FORCEINLINE. I also assume that at processing line 8359 __builtin_offsetof is already defined. I need to look at the windows stuff, please push your latest tree, whatever state it is. I can that also refer to a particular commint, if I see something. Thank you.

The code compiled during go generate AFAIK already includes a `#include <builtin.h>' but as
we previously noted, the compiler doesn't seem to be very keen to use that here.

When go generate in ccir/libc runs, builtin.h is not used when generating .h files. It's used in emit() to sanity check the just generated header, because that generated header will be later used by ccir tests while including <builtin.h>. The same is done by generator.go in cznic/sqlite.

I'm trying to add two new headers (process.h, windows.h), which depend on other builtin's,
which is why go generate doesn't work again. The current stuff for CCIR is commited.
When figuring that out, I might be able to remove some temporary solutions from predefined.h
also.
FORCEINLINE is effectively "" (and occurs on 7000 lines before which don't seem to error).
go run generator.go returns https://gist.github.com/steffengy/906287910332c7dab2e5278724fce6a9

Thanks. Looking at the tree and the gist, nowhere I can find the definition of FIELD_OFFSET. That would explain the unexpected typedefname NT_TIB, expected optional argument expression list or one of [&&, '!', '&', '(', ')', '*', '+', '-', '~', ++, --, _Alignof, character constant, floating-point constant, identifier, integer constant, long character constant, long string constant, sizeof, string literal] error.
Maybe you can try adding #define FIELD_OFFSET(x, y) __builtin_offset(x, y) to predefine.h?

@cznic
It's already expanded as I read the log?
Atleast the last line
inline  struct _TEB *NtCurrentTeb(void) { return (struct _TEB *)__readgsqword(((LONG) __builtin_offsetof(NT_TIB, Self))); }

indicates that:
https://gist.github.com/steffengy/906287910332c7dab2e5278724fce6a9#file-huge_log-txt-L7379
Adding your suggestion to predefined.h:
cannot redefine macro FIELD_OFFSET: argument names differ

It's already expanded as I read the log?

You're right. I forgot the -ccp does not show #defne stuff :-(

cannot redefine macro FIELD_OFFSET: argument names differ

What I suggested was nonsensical. FIELD_OFFSET is obviously defined.

Unfortunately for this case I cannot simply workaround it in predefined.h
since there're quite a few builtuns (and because of the conflicts mentioned above),
so this time we'll very likely have to get to solve this builtin.h issue.
I still don't understand how that works for the linux builds.
I guess there should be references to __builtin stuff too,
why does it work there but fails here?

I still don't understand how that works for the linux builds.

I will try to summarize how I think it works. Sorry for being somethimes wrong, I hope this time it's more correct.

generator.go first slurps cpp -dM. No .h stuff is used from ccir/libc. Acquired defintions are written to predefined_goos_goarch.h

generator.go attempts to generate ccir/libc headers for those listed here. When generating a ccir/libc header, <predefined.h> pulls the definitions created in step 1, but any header included is the system one as we need to get the system defintions for things in stdio.h, stdlib.h etc.

According to the mre (macro re) and dre (definition re) regexps, stuff is selected from the resulting AST from step 2. and written to the appropriate foo_goos_goarch.h in ccir/libc

What was just created, foo_goos_goarch.h is compiled again, this time using <builtin.h> provided in ccir/libc, effectively testing if it's gonna work later when really used without GCC.

If successful, the respective foo_goos_goarch.go file is written containg all the constants just seen in step 4. Here is a missing piece, it should generate also types for ccgo. Not implemented yet.

Later, in ccir tests and in sqlite generator.go, the situation when compiling C is the same as in step 4. It forces including <builtin.h> that pulls <predefined.h>, rest is the C input, whatever it is.

... I guess there should be references to __builtin stuff too,
why does it work there but fails here?

__builtin_offsetof and friends are defined in builtin.h. On Linux GCC builtins are what the name suggest, they're built in, no need to define them so in step 2 they're known. When later doing ccir tests or sqlite go generate, we supply them from <builtin.h> because we are not GCC and cznic/cc has no builtins. (It has some which are not possible to define in a .h file.)
That's seems to explain how it works on Linux. Something is different [with GCC] on Windows.
I hope I reconstructed what the code does correctly...

Errata: The regexps selects the whole file where the match was found, not a single macro/definition.

Compiling the following using MINGW-GCC also works:
#include <stdio.h>

typedef struct {
    int a;
    int b;
} Test;

int main() {
    printf("%d", __builtin_offsetof(Test, b)); // 4
}

Grepping through the all MINGW files I also find no definition of __builtin_offsetof,
so I conclude it's also a GCC-builtin and very similar (or the same) as linux.
How are they known in step 2. under linux?
AFAIK they aren't defined in predefined_linux_amd64, so how does cznic/cc
know about them? Does it somehow ask GCC about them?
I think understanding that might help here to figure out what's going wrong.

How are they known in step 2. under linux?

I believe on Linux GCC knows __builtin_offsetof because it's hardcoded/predefined directly in the compiler and no "includable" definition of it exists, even that in this case it can exist. Some builtins are not possible to define in a .h file, that's why I think cpp -dM does not show any builtins.
The question is why GCC on Windows in step 2 does not know the same predefined builtin. My guess is that on Windows GCC is maybe trying to be compatible with C sources written for MS C/C++ compilers and the builtin has a different name. But no googling confirms that so far to me.

Compiling the following using MINGW-GCC also works:

That proves the builtin is known actually.

Yeah my question is how cznic/cc thinks it's defined on linux, when it's not in cpp -dM (and therefore not in predefined_linux_amd64.h). Does it do some kind of other call to GCC to ask about what builtins its aware of, or how does it accomplish to resolve them on linux?
Or is step 2. actually "compiled" using GCC itself, that'd be unlikely since it's going through cznic/cc
as far as I skimmed the code, so I just assume that's not the case.

AFAIK they aren't defined in predefined_linux_amd64, so how does cznic/cc
know about them? Does it somehow ask GCC about them?

cznic/cc does not know __builtin_offset, it comes from <builtin.h> in ccir/libc, but not in step 2. Which means I'm now lost, I do not understand how step 2. works :-( It does work here, just tried with your latest tree.
Perhaps, when generating a ccir/libc header on Linux, those macros are never used but on Windows they are?

Or is step 2. actually "compiled" using GCC itself, that'd be unlikely since it's going through cznic/cc
as far as I skimmed the code, so I just assume that's not the case.

No, it's cznic/cc which is used, you're right.

Perhaps, when generating a ccir/libc header on Linux, those macros are never used but on Windows they are?

The more I think about this, the more it seems plausible to me. If it is so, the lack of them on Linux in step 2. would simply not be a problem. But it is and you Windows port unearthed it.

Perhaps, when generating a ccir/libc header on Linux, those macros are never used but on Windows they are?

possible, sure. A quick grep into linux include's might shed some light onto this.
Is it possible that we need to call .gccEmu for cznic/cc or is that for someother use case?

Is it possible that we need to call .gccEmu for cznic/cc or is that for someother use case?

 I dont believe it can help us. But in a sense it was a precursor of builtin.h in ccir/libc. Kind of.
You've already put some defintions into predefined.h, so maybe just carefully moving required definitions from builtin.h to predefined.h is the way to go?
But I have a feeling the split between builtin.h and predefined.h was necessary otherwise things fell apart on Linux. Anyway, can you please try it? There must be a proper solution somewhere. Exhaustive search is a painful way how to find it, but I have no better idea.

Adding to predefine.h work so far for __builtin_va_start and __builtin_va_end
(They temporarily are there since one of the first commits)
Adding #define __builtin_offsetof(st, m) ((__SIZE_TYPE__)(&((st *)0)->m)) to predefine.h somehow results in a bulk of additional definitions:
diff --git "a/stddef_windows_amd64-4e51852.001.h" "b/stddef_windows_amd64.h"
index 6575ab8..058e839 100644
--- "a/C:\\Users\\steff\\AppData\\Local\\Temp\\TortoiseGit\\stddef_windows_amd64-4e51852.001.h"
+++ "b/D:\\projects\\Go\\gopath\\src\\github.com\\cznic\\ccir\\libc\\stddef_windows_amd64.h"
@@ -32,6 +32,9 @@ errno_t _set_errno (int _Value );
 errno_t _get_errno (int *_Value );
 extern unsigned long __threadid (void );
 extern uintptr_t __threadhandle (void );
+typedef char *__builtin_va_list ;
+typedef __builtin_va_list va_list ;
+typedef void *FILE ;
 #define _INC_CRTDEFS
 #define _CRT_PACKING (8)
 #define _CRTNOALIAS
@@ -88,3 +91,25 @@ extern uintptr_t __threadhandle (void );
 #define __STDDEF_H__
 #define NULL ( ( void * ) 0 )
 #define offsetof(TYPE, MEMBER) __builtin_offsetof ( TYPE , MEMBER )
+#define _PREDEFINED_H_
+#define __str(x) # x // LINE 95
+#define ____header(name, os, arch) __str ( name ## _ ## os ## _ ## arch . h )
+#define ___header(name, os, arch) ____header ( name , os , arch )
+#define __header(name) ___header ( name , __os__ , __arch__ )
+#define _MSC_VER (1300)
+#define __restrict__
+#define __int32 long
+#define __int64 long long
+#define _INC_STDARG
+#define __builtin_va_start(ap, arg) ap = ( __builtin_va_list ) ( & arg )
+#define __builtin_va_end(ap) ap = 0
+#define __builtin_offsetof(st, m) ( ( __SIZE_TYPE__ ) ( & ( ( st * ) 0 ) -> m ) )
+#define _VA_LIST_DEFINED
+#define __inline inline
+#define __forceinline inline __attribute__ ( ( __always_inline__ ) )
+#define __USE_MINGW_ANSI_STDIO (0)
+#define index(s, c) strchr ( s , c )
+#define rindex(s, c) strrchr ( s , c )
+#define _FILE_DEFINED
+#define __unaligned
+#define _X86INTRIN_H_INCLUDED

So somehow this causes e.g. __str to be pulled in, which results in things
falling apart like:
stddef_windows_amd64.h:95:9: cannot redefine macro using a replacement list of different length
stddef_windows_amd64.h:96:9: cannot redefine macro using a replacement list of different length
stddef_windows_amd64.h:97:9: cannot redefine macro using a replacement list of different length
stddef_windows_amd64.h:98:9: cannot redefine macro using a replacement list of different length
stddef_windows_amd64.h:99:9: cannot redefine macro using a replacement list of different length
stddef_windows_amd64.h:104:9: cannot redefine macro using a replacement list of different length
stddef_windows_amd64.h:106:9: cannot redefine macro using a replacement list of different length
stddef_windows_amd64.h:109:9: cannot redefine macro using a replacement list of different length
stddef_windows_amd64.h:110:9: cannot redefine macro using a replacement list of different length
stddef_windows_amd64.h:111:9: cannot redefine macro using a replacement list of different length
15:33:39.233141 global.go:205: too many errors

The errors itself are quite strange, not sure why the replacement list should have a different length.
(In the best case this would just be some bug and without it redefinition would work)
But I guess even if I somehow manage to fix this I have a feeling that there might be more to pop up.
(windows.h and the dependant headers are quite big, and since it's windows expected
to do "unexpected" stuff)

Sad news.
FTR: Using https://github.com/cznic/ccir/commit/4e518522a97d1220eb4a3f45b8b01cb21e0afaa1 as a base and moving the defintion of __builtin_offsetof from builtin.h to predefine.h (https://github.com/cznic/ccir/commit/030b115d43286f40e7d1bd7d5eda955d1470f12f) results in:
jnml@4670:~/src/github.com/cznic/ccir/libc$ go generate
15:51:37.848794 generator.go:119: stddef_linux_amd64.h:81:9: cannot redefine macro using a replacement list of different length
stddef_linux_amd64.h:82:9: cannot redefine macro using a replacement list of different length
stddef_linux_amd64.h:83:9: cannot redefine macro using a replacement list of different length
stddef_linux_amd64.h:84:9: cannot redefine macro using a replacement list of different length
stddef_linux_amd64.h:85:9: cannot redefine macro using a replacement list of different length
exit status 1
libc.go:2: running "go": exit status 1
jnml@4670:~/src/github.com/cznic/ccir/libc$

So unfortunately, it does break Linux, I recalled that correctly. Anyway, now I have a broken Linux branch, which I can investigate independently from having Winows. At least something.

That's the same issue I'm having on windows.
I first though that because predefine.h is included before that
#define __str(x) #x would expand to #define #x #x, but that would throw a different error.
Hopefully now that we both can look into it, we can figure something out

This fixes the Linux buid
diff --git a/libc/generator.go b/libc/generator.go
index 229b106..e604d77 100644
--- a/libc/generator.go
+++ b/libc/generator.go
@@ -847,7 +847,7 @@ func extractCopyright(f string) string {
 }

 func skip(f string) bool {
-       return strings.HasPrefix(f, "predefined_") || f == "<predefine>"
+       return strings.HasPrefix(f, "predefined") || f == "<predefine>"
 }

Please try the same on Windows.

I get a different error, which seems to be unrelated, so I say very likely yes.
Still a bit funny that a possible typo is the issue of this^^

I get a different error, which seems to be unrelated, so I say very likely yes.

Yes, like in 'it fixes the __builtin_offsetof problem on Windows'? :smile:

Yeah now it seems to related to some other MINGW specific intrinsic stuff...
Looking at ifdef's/include to figure out what I need to do and similar stuff (still the same line but __readgsqword this time)

Great! Every progress counts.

__MINGW_INTRIN_INLINE void __cdecl __debugbreak(void)
{
  __asm__ __volatile__("int {$}3":);
}

It seems strange to me that something like this is included now.
If I understand the VM correctly, I'd doubt that assembly instructions
would even work like that? (or are they also "translated"?)

No, assembler is not supported and there are no intentions to do so. But the generated headers have no function bodies, so this should be harmless. Or does it trigger some problem?

It seems to fail during parsing (probably because EnableAsm() / EnableAlternativeKeywords() isn't specified).
The actual problem is that I need a definition of __readgsqword.
__debugbreak (and maybe other [likely unreferenced] assembly-intrinsics (including __readgsqword which seems to use gs/segment registers using assembly instructions) are pulled in by setting __MINGW_INTRIN_INLINE.
So I'm wondering if it'd make sense to define __readgsqword  as an external builtin,
which I then would define in virtual.h, do you agree that this would be the correct way
to handle such builtins?

From here:

These intrinsics are only available in kernel mode, and the routines are only available as intrinsics.

We surely should not be in kernel mode, me thinks.

So I'm wondering if it'd make sense to define __readgsqword as an external builtin,
which I then would define/handle in cznic/virtual, do you agree that this would be the correct way
to handle such builtins?

Regardless of the kernel mode mystery, the VM has no concept of the GS segment register, so I don't see what the builtin could possibly do.
We can surely enable the assembler and tweak ccir to translate any assemble containing function to a panic instruction, hoping nothing will ever call it. If it happens anyway, there will be a stack trace so we can see where/why it happens and what to do about it. I don't expect SQLite to do anything which ends in calling this intrinsic. WDYT?

The documentation seems to be wrong there.
MSVC TLS (as in thread local storage) uses the GS segment.
Reference
Nevertheless I don't expect it to be actually called anytime by SQLITE,
it's just necessary because sqlite3.c -> windows.h  -> winnt.h which defines
functions, which could potentially be used like:
 FORCEINLINE struct _TEB *NtCurrentTeb(VOID) { return (struct _TEB *)__readgsqword(FIELD_OFFSET(NT_TIB,Self)); }

Potentially I see multiple solutions

Your proposed assembly+panic solution  which would allow __MINGW_INTRIN_INLINE stuff
to be defined (and therefore will likely cover most of those)
Stubbing the bare-minimum in predefined.h to panic in some kind
Stubbing the bare-minimum in predefined.h and handling (panicing) them in cznic/virtual

What would be a good solution in your opinion?

No strong opinion here. Feel free to chose what you feel is better or easier. Stubbing a panic (2) using a define can use the __builtin_abort, FYI.

I think 2. seems like least work.
The only issue I see is that I would want to include the called function so if it should actually
happen you can figure out why. Is there a possibility so somehow given __builtin_abort an abort reason? (Something like "__readgsqword stub called")

Is there a possibility so somehow given __builtin_abort an abort reason? (Something like "__readgsqword stub called")

No, but you can copy and adjust the assert macro. It decorates the fail using __func_ and __LINE__. And the VM panic per se should report something useful. (A build tag may be necessary for that, not sure.)
Additionally, FTR, there is a TLS in cznic/virtual: https://github.com/cznic/virtual/blob/769db7bb1f04e2dfad6964c3e4c0961044f4860f/thread.go#L70
If needed, it can be extended or even made run-time configurable in its size etc.

Also, if a builtin is registered, but not dispatched in the giant switch case in vm/cpu.go, there should be also an instruction trap with stack trace citing the opcode (== the builtin) name.

The generator starts to be really annoying because it doesn't output in the correct order (typedefs are after the type usages) and the workaround grows:
https://github.com/cznic/ccir/commit/cadbea0e7f830d3138fa5fd3fbbe04280e26b362
Unfortunately this workaround might not work anymore (currently struggling with rpc.h, rpcdce.h)  since there're so many cross-dependencies.
Any idea if there's a better solution?

The generator starts to be really annoying because it doesn't output in the correct order (typedefs are after the type usages) and the workaround grows:

Do not workaround that. If it's a bug it has to be fixed. But I recall experiencing something similar and the fix was as simple as to add the type name in question to the respective regexp.
Long version: We rely on that the GCC headers include, define and use/refer to things in the correct order (define before use, except where the language somehow permits that [typedef struct foo foo;]). Adding the required type to the dre regexp parameter of header in generate.go should simply include that definition (and its whole source file) in the preprocessing, ie. correct order. Please try and let me know the outcome.
I see you're already doing maybe exactly that. Then it may be necessary to remove any forced includes. I recall falling into that trap as well. The lesson learned was: do not add any #include unless no other way around. Using the regexp to get the definition in the correct order worked except for I think one single case (forgot where).
Sorry for the troubles. The above will hopefully improve things.

Removed the workaround entirely.
It seems like the ordering doesn't work though:
Example for FCNTL (since thats currently what fails for me):
[...]
_off64_t lseek64 (int fd ,_off64_t offset ,int whence );
[...]
typedef long long _off64_t ;

{"fcntl", "F_WRLCK", "_off64_t|__off_t|open|struct flock|__time32_t"},

Adding a println like here results in the correct ordering on the console
(without the filtering applied, logically, so I assume there's something wrong with that):
diff --git "a/src\\github.com\\cznic\\ccir\\libc\\generator.go"
index ea52de1..9d7824a 100644
--- "a/generator-cadbea0.002.go"
+++ "b/\generator.go"
@@ -956,6 +956,7 @@ func header(nm, mre, dre string) {
 			// nop
 		case 1: // Declaration                  // Case 1
 			d := declaration(n.Declaration)
+			fmt.Println(d)
 			if dbg || re.MatchString(d) {
 				if !m[f] {
 					m[f] = true
@@ -972,32 +973,6 @@ func header(nm, mre, dre string) {
 		}
 	}

Note the dbg || .... It reminds me that when solving probably exactly the same problem on Linux, I added that option to verify the respective type definition really comes before it's use. Because if that's the case and the definitions from the appropriate files are later on emitted in the discovery order (fo = append(fo, f)) everything should just work. IOW, please try go run generate.go -debug, it may show something useful. Using that option should define more than necessary and can introduce other bugs, but it should hopefully allow to verify if the define/use order is correct before we output to libc/foo.h. If the input order is correct, we just need to find why/where we do not respect that order on output, which should be a simpler problem.
There's an exception to the concept above. Macros, which can effectively define/name/map types, have to be emitted after other definitions. There are other problems when keeping them in the original order. Not sure ATM what was the reason, but I think that was one of the last tweaks needed before finally the generating started to work on Linux.
Please experiment using he above hints/thoughts. The solution, once you find it, will be probably simple. And surely my fault ;-)
PS: I know I've just reiterated parts of your earlier reasoning. It helps me to write it down anyway because formulating thoughts sometimes reveal the related mistake.

go run generate.go -debug generates the wrong order, as before.
I think the issue is that we output ordered by files, which might not be correct
for cross-dependencies.
(Atleast that's what I get from adding the fmt.Println(d) mentioned previously)
I'll try to refactor that to order by declaration-order and we'll see what happens

go run generate.go -debug generates the wrong order, as before.

Surprising, but if that means there's a bug in generator.go, it's good to have this piece of information. But you've earlier said the using the debug print the order was okay. Using -debug should include everything in original, preprocessing order. I must be missing something.

I think the issue is that we output ordered by files, which might not be correct
for cross-dependencies.

Hmm, now I think you've nailed it. Source files order is not the same as the order the compiler sees after preprocessing b/c #includes in C are quite powerful and not limited to effectively include the whole file or nothing. GNU libc actually uses things like #define need_foo before including things to include very specific parts of other headers.
So it start to look like we need to record the original order in full and "and" that with the selected-by-regexp property but still output in the same order. That will probably also remove the macros-after-defintions wart.
Do you feel like implementing this particular change or do you want me to have a stab on it?

I'll try to refactor that to order by declaration-order and we'll see what happens

Ah, you already decided on that ;-)

Once again, I'm sorry you have to deal with my bad and buggy code. I'm very grateful you are helping in improving it!

BTW: Emitting just everything in incoming order would by my first attempt. If that, by chance, works, I see no real reason why not to keep that solution.The final IR/binary does in no way contain the potentially redundant definitions so it would not hurt anything, I think. (Just a 2c thought about the simplest implementation.)

Making those changes (not final) fixes it for fcntl.h:
diff --git "a/generator-cadbea0.005.go" "b/generator.go"
index ea52de1..14bdc34 100644
--- "a/generator-cadbea0.005.go"
+++ "b/generator.go"
@@ -943,6 +943,7 @@ func header(nm, mre, dre string) {
 	re := regexp.MustCompile(dre)
 	dset := map[string]scanner.ErrorList{}
 	var fo []string
+	i := 0
 	for l := ast; l != nil; l = l.TranslationUnit {
 		n := l.ExternalDeclaration
 		pos := position(n)
@@ -962,6 +963,10 @@ func header(nm, mre, dre string) {
 					fo = append(fo, f)
 				}
 			}
+			// make sure we can sort by declaration order later
+			pos.Filename = ""
+			pos.Line = i
+			i += 1
 			dset[f] = append(dset[f], &scanner.Error{Pos: pos, Msg: d})
 		case 2: // BasicAssemblerStatement ';'  // Case 2
 			log.Fatalf("%s: TODO %v", pos, n.Case)
@@ -1024,6 +1003,7 @@ func header(nm, mre, dre string) {

 	var a, more []string
 	cp := map[string]bool{}
+	aggregatedDecls := scanner.ErrorList{}
 	for _, f := range fo {
 		if s := extractCopyright(f); s != "" {
 			cp[f] = true
@@ -1034,14 +1014,11 @@ func header(nm, mre, dre string) {
 %s
 `, f, s))
 		}
-		set := dset[f]
-		set.Sort()
-		for _, v := range set {
-			if *oPos || *oDebug == nm {
-				v.Msg = fmt.Sprintf("%s // %s", v.Msg, v.Pos)
-			}
-			a = append(a, v.Msg)
-		}
+		aggregatedDecls = append(aggregatedDecls, dset[f]...)
+	}
+	aggregatedDecls.Sort()
+	for _, decl := range aggregatedDecls {
+		a = append(a, decl.Msg)
 	}
 	for _, f := range fo {
 		if !cp[f] {
@@ -1101,7 +1078,7 @@ func main() {
 		{"ctype", "TODO", "tolower|__int32_t|pthreadlocinfo"},
 		{"dlfcn", "RTLD_NOW", "dlsym"},
 		{"errno", "EINTR|ETIMEDOUT", "errno"},
-		{"fcntl", "F_WRLCK", "open|struct flock|__off_t|__time32_t|_off64_t"},
+		{"fcntl", "F_WRLCK", "_off64_t|__time32_t|open|struct flock|__off_t"},
 		//TODO{"float", "TODO", "TODO"},
 		{"limits", "INT_MAX", "TODO"},
 		//TODO{"locale", "TODO", "TODO"},

The only issue I see with this is handling the copyright information since we cannot
just insert them at file boundaries can we?
While writing this I actually have a potential solution of inserting them at the top of the file and referencing them at bounds which might be best illustrated with an example:
[COPYRIGHT_1] Copyright Super Cool Corporation ...
[COPYRIGHT_2] Copyright XYZ! DO THAT! DONT DO THAT ....

// BOUNDARY: BELOW HERE [COPYRIGHT_1] TILL NEXT BOUNDARY:
int company_1_test();
int company_1_test2();
// BOUNDARY: BELOW HERE [COPYRIGHT_2] TILL NEXT BOUNDARY:
int company_2_test();
int company_2_test_2();
// BOUNDARY: BELOW HERE [COPYRIGHT_1]
int company_1_test();
// BOUNDARY: BELOW HERE [COPYRIGHT_2]
int company_2_test();

Any idea about that?

Making those changes (not final) fixes it for fcntl.h:

Great! Please push the changes, I want to cross-check for Linux, thank you.

The only issue I see with this is handling the copyright information since we cannot
just insert them at file boundaries can we?

No problem with that.

While writing this I actually have a potential solution of inserting them at the top of the file and referencing them at bounds which might be best illustrated with an example:

[COPYRIGHT_1] Copyright Super Cool Corporation ...
[COPYRIGHT_2] Copyright XYZ! DO THAT! DONT DO THAT ....

// BOUNDARY: BELOW HERE [COPYRIGHT_1] TILL NEXT BOUNDARY:
int company_1_test();
int company_1_test2();
// BOUNDARY: BELOW HERE [COPYRIGHT_2] TILL NEXT BOUNDARY:
int company_2_test();
int company_2_test_2();
// BOUNDARY: BELOW HERE [COPYRIGHT_1]
int company_1_test();
// BOUNDARY: BELOW HERE [COPYRIGHT_2]
int company_2_test();
Any idea about that?

No problem either, but sounds like more work.
Reproducing the copyright notices is very important. Having them sorted and/or grouped near the top of the file is nice and somewhat preferable, but not important. Please use any simple solution. We can improve on this later, but we cannot distribute the generated headers without the proper attribution. (I am not a lawyer, but that's what I think.)

Yeah I'll implement it and push it, doesn't sound like too much work.
Maybe give me a couple  minutes :)

No hurry. You're already near c ;-)

@cznic Sorry took a bit longer than expected, should work though, let me know :)

All indicators green on Linux!

Nice! windows.h also is miles further than before :)

panic: windows_windows_amd64.h:476:9: TODO

goroutine 1 [running]:
github.com/cznic/cc.(*StructOrUnionSpecifier).isCompatible(0xc04500bf20, 0xc04500bda0, 0x7c3d8)
        D:/projects/Go/gopath/src/github.com/cznic/cc/ast2.go:3394 +0x577

Is CASE1 a trivial case or something that requires quite a bit of logic?
Since CASE0 doesn't seem to check the SpecifierQualifierList, I guess it's trivial?
https://github.com/cznic/cc/blob/master/ast2.go#L3362-L3394

Is CASE1 a trivial case or something that requires quite a bit of logic?

Not sure yet, on it.

Please push the windows_windows_amd64.h file, thanks. I cannot imagine how we can hit case 1. It's probably yet another thing I don't know about C :open_mouth:

Hmm, it must be an anonymous field, right?

Quite possible.

Fix attempt https://github.com/cznic/cc/commit/3c633d52f0d0a5db61b46d58ddb18a879075c339 pushed. Please try it.

panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xc0000005 code=0x0 addr=0x8 pc=0x539be1]

goroutine 1 [running]:
github.com/cznic/cc.(*SpecifierQualifierList).isCompatible(0xc0481822d0, 0xc04813a720, 0x40b291)
        D:/projects/Go/gopath/src/github.com/cznic/cc/ast2.go:3117 +0x51
github.com/cznic/cc.(*StructOrUnionSpecifier).isCompatible(0xc0485b5a40, 0xc0485b5920, 0x7c3ca)
        D:/projects/Go/gopath/src/github.com/cznic/cc/ast2.go:3409 +0x49d
github.com/cznic/cc.(*Bindings).defineStructTag(0xc044fe35f0, 0x2caf4c0000e021, 0x7c3ca, 0x7434e0, 0xc0485b5a40, 0xc046258660)

Just return true for now (shame on me): https://github.com/cznic/cc/commit/563948c33bbc4e7373939ff23637f66cc3131cd0

Haha I have a feeling the windows headers are hitting quite some unusual cases:
panic: windows_windows_amd64.h:873:9: TODO

goroutine 1 [running]:
github.com/cznic/cc.(*StructOrUnionSpecifier).isCompatible(0xc043575920, 0xc043575800, 0xa7c0d)
        D:/projects/Go/gopath/src/github.com/cznic/cc/ast2.go:3397 +0x347```

I'll add a .isCompatible to StructDeclarator and StructDeclaratorList and refactor it...

Collaborator's invite for cc on the way ;-)

Warning: Writing cc, a year or so ago, was for me actually a way to start learning C.  It's original purpose was to handle just and only exactly one file, nothing else was originally important. The name of that file was/is sqlite3.c ;-)

I'll add a .isCompatible to StructDeclarator and StructDeclaratorList and refactor it...

Appreciated, thank you!

Done: https://github.com/cznic/cc/commit/01bc451bc7f1cba70812acfcc275f8c4395841ac
could you take a look at this since the following comment IMO was wrong and was replaced with
the correct one:
// StructDeclaratorList ',' StructDeclarator  // Case 1
The tests don't seem to show any change to master.

could you take a look at this since the following comment IMO was wrong and was replaced with
the correct one:

I found one comment which is probably incorrect, but I'm not sure where the change you refer to is located?

// StructDeclaratorList ',' StructDeclarator // Case 1

Please link to the diff line, thank you.

https://github.com/cznic/cc/commit/01bc451bc7f1cba70812acfcc275f8c4395841ac#diff-69cc9cc7ea094f7c11d142b0a77d423cL3396
Probably the same origin, that's the one I corrected since it was bogus IMO.

Yeah, that was wrong AFAICT. Good catch, I've got confused by that today already.

Would it be difficult to support anonymous unions as  struct members in cc?
typedef struct _OVERLAPPED {
  ULONG_PTR Internal;
  ULONG_PTR InternalHigh;
  union {
    struct {
      DWORD Offset;
      DWORD OffsetHigh;
    };
    PVOID  Pointer;
  };
  HANDLE    hEvent;
} OVERLAPPED, *LPOVERLAPPED;

(from MSDN directly but identical to MINGW definition)
SQLite accesses e.g. _OVERLAPPED.Offset

Should be possible via EnableAnonymousStructFields.
If that option is already used, then we have another problem to solve.

Maybe the problem is the selector per se, not able to find the field...? (TODO in cc perhaps)

EnableAnonymousStructFields is already specified for sqlite.
Any idea where I'd have to look to find that selector & relevant stuff?

Found it: https://github.com/cznic/cc/blob/master/etc.go#L976

By accident I also noticed this is actually wrong in the generated header:
typedef struct _OVERLAPPED {ULONG_PTR Internal ;ULONG_PTR InternalHigh ;union {struct {DWORD Offset ;DWORD OffsetHigh ;}s ;PVOID Pointer ;}u ;HANDLE hEvent ;}OVERLAPPED ;

seems like some IFDEF is missing...

Found it: https://github.com/cznic/cc/blob/master/etc.go#L976

Yep. It's a flat search and it should be BFS tree one actually. I think C99 describes the same resolution algorithm as Go: The shortest unambiguous path wins.

Adding _MSC_EXTENSIONS fixed the wrong generated code, problem persists.
I'll probably just implement a recursive search into all unnamed members and
throw an error when there are more than one results, does that sound okay?

'll probably just implement a recursive search into all unnamed members and
throw an error when there are more than one results, does that sound okay?

It does as that probably covers 99% cases.

However, I'm afraid ccir probably does not know yet how to handle foo.bar for bar being a member of anonymous struct/union field of foo.

I pushed a commit, I'll see if it causes any issues... I hope it's enough

Finding the member is one part of the problem. I guess the member's .Offset is not okay. It's correct within its parent anonymous struct but not necessarily correct within its non-anonymous super-parent struct if the anonymous parent has more than one field and the field found is not the first one. (Which it often is.)
Just saying that I have a feeling there's unfortunately more to this that has to be done, probably both in cc and ccir.

I pushed a commit, I'll see if it causes any issues... I hope it's enough

Thanks a lot.

FYI:
jnml@4670:~/src/github.com/cznic/cc$ git pull
remote: Counting objects: 6, done.
remote: Compressing objects: 100% (6/6), done.
remote: Total 6 (delta 1), reused 0 (delta 0), pack-reused 0
Unpacking objects: 100% (6/6), done.
From github.com:cznic/cc
 + 01bc451...520afcb win_support -> origin/win_support  (forced update)
Auto-merging ast2.go
CONFLICT (content): Merge conflict in ast2.go
Automatic merge failed; fix conflicts and then commit the result.
jnml@4670:~/src/github.com/cznic/cc$

yeah i fixed that typo, so you'll probably have to resolve that wrong comment (or reset to a commit before & pull)

yeah i fixed that typo, so you'll probably have to resolve that wrong comment (or reset to a commit before & pull)

NP, thanks again.
Morning alarm in ~6 hours, have to force myself to bed now. I'll try to watch for any Github notifications, but I can process them most likely not before tomorrow evening.

Yeah same for me...
%v ......\sqlite.org\sqlite-amalgamation-3180000\sqlite3.c:40170:5: internal error: struct _OVERLAPPED

I solved the anonymous union feature in my cznic/cc branch by transforming
a.b to *(ty*)((&b)+offset) on the AST level.
This only happens when it doesn't find a direct member and it can find such a field
using a recursive search. Hopefully the offset calculation is correct, but afaik it should.
The transformation happens on the AST (during constant-evaluation afaik).
It was the simplest way I could think of for implementation.
SQLITE now builds and links.
So basically only the OPCodes are missing now (stubs are added in the cznic/virtual branch).
Now finally getting the 2.5 hours of sleep I've left till work :)

I solved the anonymous union feature in my cznic/cc branch by transforming
a.b to (ty)((&a)+offset) on the AST level. (where offset is the entire offset of b)
This only happens when it doesn't find a direct member and it can find such a field
using a recursive search. Hopefully the offset calculation is correct, but afaik it should.

Nice, looking forward testing it.

SQLITE now builds and links.
So basically only the OPCodes are missing now (stubs are added in the cznic/virtual branch).

Great! So you should already get a VM stacktrace pointing to the first unimplemented builtin encountered. Also, running go generate in cznic/sqlite should now work.

Now finally getting the 2.5 hours of sleep I've left till work :)

Ouch ;-)

Adding EnableWideBitFieldTypes() to sqlite's generator.go allows go generate to work, yes.
generator.go:365: code 0x0003c979, text 0x00006c90, data 0x00006af0, bss 0x000004b0, pc2func 1930, pc2line 50253, symbols 302, gz 627417

Filesize of bin_windows_amd64.go is 4 bytes bigger than linux, so reasonable.
Some questions:

AFAIK virtual doesn't support multithreading yet right? (there doesn't seem to be a possibility to create an actual new thread, atleast pthread_create doesn't exist)
I just noticed because of an SQLite assertion that CCIR does define long with size==8,
while on windows size==4 is expected. I changed that in ccir/model.go but I'm
getting some strange error from cznic/ir:

Verify (A): mismatched types, got int32, expected int64

https://github.com/cznic/ccir/commit/bcbdb5abd7344d8c0c32709b05604ce68f9fde1c
any idea what else should be done to change the size?

Adding EnableWideBitFieldTypes() to sqlite's generator.go allows go generate to work, yes.

generator.go:365: code 0x0003c979, text 0x00006c90, data 0x00006af0, bss 0x000004b0, pc2func 1930, pc2line 50253, symbols 302, gz 627417

Filesize of bin_windows_amd64.go is 4 bytes bigger than linux, so reasonable.

Very nice!

AFAIK virtual doesn't support multithreading yet right? (there doesn't seem to be a possibility to create an actual new thread, atleast pthread_create doesn't exist)

It does not because sqlite has never tried to call this builtin so it's not yet implemented. However, every sql.DB connection runs in it's own VM thread.

I just noticed because of an SQLite assertion that CCIR does define long with size==8,
while on windows size==4 is expected. I changed that in ccir/model.go but I'm
getting some strange error from cznic/ir:

That will be a problem. The assumption of sizeof(long) == 4 on 32 bit archs and 8 on 64 bit archs is used in many places. I cannot find the SQLite assertion (assert.*sizeof.*long), please tell me the line number.

Verify (A): mismatched types, got int32, expected int64

The verifier is simple but just proved effective ;-)

any idea what else should be done to change the size?

I want to first check and understand why SQLite insists on 32 bit long. On second thought, it may be enough to specialize virtual/cpu64.go, ie. split it into cpu64_linux.go and cpu64_windows.go. It's the place where long ints are handled in an architecture specific way but with a unified "api".

I cannot find the SQLite assertion (assert.*sizeof.*long), please tell me the line number.

It isn't SQLite directly, rather Windows/Microsoft/MINGW let me explain:
The actual assertion that fails is assert( winSysInfo.dwAllocationGranularity>0 ); (L43634)
winSysInfo is of type SYSTEMINFO and is passed to GetSystemInfo
Then GetSystemInfo (WINAPI) populates this struct with information, assuming standard windows types.

We have a type mismatch, WinAPI writes to other offsets than we'd read from.
So we only get total non-sense back (In this case the assertion fails with getting 0, luckily).
In summary, all windows code will assume SIZEOF(LONG)==4 on every platform directly or indirectly one way or another.

In summary, all windows code will assume SIZEOF(LONG)==4 on every platform directly or indirectly one way or another.

Okay, the virtual/cpu64.go split is IMO the starting point. Let's see if I have properly routed every long access through the {read,write}[U]long routines. Near the top is a longbits constant, need to be changed for Windows as well.

Okay, but isn't IR running before VM? So here we have a codegen problem?
sqlite3.c:22340:7 all_test.go:823: # [287]: Verify (A): mismatched types, got int32, expected int64

https://github.com/cznic/ir/blob/master/operation.go#L522

Did the split between cpu64.go and cpu64_windows.go c699e43ee86a9c472a26672f762e5140dd54da91.
The IR error still persists, still wondering if it's related to wrong codegen (seems like a uint32 = uint64 convert) or something else.
all_test.go:823: # [287]: Verify (A): mismatched types, got2 int32, expected int64
 memsys5Size:0x2e:       convert                 int64, int32    ; sqlite3.c:22340:7
  # [287, err]: *ir.FunctionDefinition {sqlite3.c:22337:1 memsys5Size func(*struct{})int32 InternalLinkage } [p]

Any idea?
PS
just running go test -run TestSqlite3 at the moment since GCC(?) tests etc. seem to fail indeed in the vm
unexpected fault address 0x102aaff68
fatal error: fault
[signal 0xc0000005 code=0x0 addr=0x102aaff68 pc=0x648252]

goroutine 316 [running]:
runtime.throw(0x75a480, 0x5)
        Go/src/runtime/panic.go:596 +0x9c fp=0xc042a18670 sp=0xc042a18650
runtime.sigpanic()
        Go/src/runtime/signal_windows.go:164 +0xff fp=0xc042a186a0 sp=0xc042a18670
github.com/cznic/virtual.(*cpu).run(0xc04208c700, 0x2, 0x0, 0x0, 0x0)
        cznic/virtual/cpu.go:987 +0x2202 fp=0xc042a190a8 sp=0xc042a186a0
github.com/cznic/virtual.New(0xc0423f4780, 0xc042414dd0, 0x1, 0x1, 0x8ab020, 0xc04230fab0, 0x8ac9a0, 0xc042004b18, 0x8ac9a0, 0xc042004b20, ...)
       cznic/virtual/virtual.go:73 +0x2de fp=0xc042a19168 sp=0xc042a190a8
github.com/cznic/virtual.Exec(0xc0423f4780, 0xc042414dd0, 0x1, 0x1, 0x8ab020, 0xc04230fab0, 0x8ac9a0, 0xc042004b18, 0x8ac9a0, 0xc042004b20, ...)
       cznic/virtual/virtual.go:84 +0xf0 fp=0xc042a19220 sp=0xc042a19168
github.com/cznic/ccir.expect1.func3(0xc0420aca00, 0x2f, 0x7734b8, 0xc04205caf0, 0x49, 0xc042a19b20, 0xc0423f4780, 0xc04230fab0, 0xc042004b18, 0xc042004b20, ...)
        cznic/ccir/all_test.go:287 +0x246 fp=0xc042a19350 sp=0xc042a19220

Okay, but isn't IR running before VM?

No, but the verifier also computes the stack structure of all possible code paths and checks that jump targets are stack-structure compatible with the jump origin stack. This error says the declared IR instruction operand type does not match the computed stack operand. That's bad, the program may easily crash when executed later.
The error should be accompanied with an IR dump of the offending function. From it I hope I can find where an invalid leak of the assumption longbits=64 on Windows is. (Assuming you've already split cpu64.go)
If it does not produce the dump, maybe running go run generator.go -log will produce it. Otherwise I'll add the option. Please put the dump anywhere for me to look at, thank you.
I hope to be at home in about one hour, will try to resolve this. It's a pitty I never contemplated checking int sizes on Windows. Or that they are more restricted than C99 prescribes. TIL

So here we have a codegen problem?

Yes, ccir produced invalid IR. It's a bug.

just running go test -run TestSqlite3 at the moment since GCC(?) tests etc. seem to fail indeed in the vm

My lesson learned was to not skip tests in ccir. They are in order of complexity (TCC, GCCExec, Sqlite) for this reason.

The IR error still persists, still wondering if it's related to wrong codegen (seems like a uint32 = uint64 convert) or something else.

I believe it is incorrect codegen.

go test -run TestSqlite > ir.dump: https://gist.github.com/steffengy/0dbe7b52a8332010d627f601b46c5afb
So do you advise first looking into that mentioned GCC-Test-Fault/Panic pasted above?

Thanks, but the memsys5Size function dump ends at PC 0x18 but the fault is at 0x2e, please paste the full dump or at least up to an including PC 0x2e.

So do you advise first looking into that mentioned GCC-Test-Fault/Panic pasted above?
(gcc.c-torture\execute\20041214-1.c)

Yes, because the sources of GCCExec tests are often small programs, so they are easier to analyze.

Sorry somehow my clipboard messed up. Updated the file.
Yeah it's just that it's a fault in a Load64 instruction (while loading the value during readI64)
so something that get's an invalid pointer on the stack.
That means to track down the origin I first have to somehow dump all OPs,
look at them and try to determine the origin, which likely is also in CCIR,
or do you have a better idea?
I could also search for all places that emit Load64 intructions and try to spot
some obvious mistake (assuming it's somewhere related to long).

All of the approaches you mention are reasonable. I will try first the IR dump, but it must unfortunately wait a bit. I'm right now pushing the new profiling API to virtual (and its use in sqlite), then I'm leaving the office for today.

Finally managed to dump the LOG output for 20041214-1.c:
0x00079         load64         0x0
0x0007a         push32         0x8
0x0007b         negindexu32    0x1
0x0007c         store64
0x0007d         load64         0x0       <------ crash

Added the full output also to the GIST link above.
Let me know if you know if that might be related or is something else.
I guess the actual problem is negindexu32 --> store64, right?

Here I am. Will report back when/if I find something.

I don't know if I have mentioned the -virtual.trace build tag before. Can be useful in chasing such crashes, but it seems to me you've discovered it already. Another help is in some cases adding -virtualTesting to $ go test in ccir.

@cznic
For the GCC case I just added a println before the actual execution (since it faults in the execution
of an opcode, which won't be outputed after afaik, atleastpassing  -tags virtual.trace to go test didn't help.)
The problem definitely is that Store64 gets some invalid input.
It writes<PHONE_NUMBER> which Load64 uses as pointer. (readI64)
Only doing a readI32 in Store64 would result in 42991464, which would be
a valid pointer.
The question is just where the input is coming from. Still have to understand
quite few OPCodes.

Thanks for more data points. So far no idea about 20041214-1.c. I want to look at the run time stack values.
Please paste $ go test -v -run GCC -re 20041214-1.c -tags virtual.trace, it shows also the top 4 or 5 stack slots, maybe it can help.
FTR: Stack slots are shown as if the item is a pointer. Stack slots are aligned at that size, but if the actutal item is smaller, the higher bits are garbage, but that is most often perfectly okay.

Still have to understand quite few OPCodes.

I feel the pain. After some months fiddling with them I can read a bit ;-)
Also, IR assembly and virtual assembly are two separate beasts, sorry.

The question is just where the input is coming from.

I can't even yet figure out why there's that negindex thing.

Windows
0x00076		push           bp-0x10
0x00077		push           ap-0x18
0x00078		dup64
0x00079		load64         0x0
0x0007a		push32         0x8
0x0007b		negindexu32    0x1

Linux
0x00076		push           bp-0x10
0x00077		push           ap-0x18
0x00078		dup64
0x00079		load64         0x0
0x0007a		push64         0x8
0x0007b		negindexu64    0x1

We need to figure out why windows does a push32 0x8 at 0x7a instead of push push64. That's the cause I believe.

On a second thought I interpret NegIndexU32 as doing some array indexing
with size 8 (op.N). So my current assumption is that either that NegIndexU32 is called
with the wrong size (8 instead of 4) or the Store64/Load64 should be a Store32/Load32.

go test -v -run GCC -re 20041214-1.c -tags virtual.trac

That doesn't output any stack slots unfortunately (since it's a fatal error: fault)
and immediately aborts (accessing an "invalid" pointer  causing a SEGFAULT)

Also, IR assembly and virtual assembly are two separate beasts, sorry.
The IR assembly is higher-level though, right?

So type-errors, like I presume here, should be easier to spot?
(Except if it's actually some translation of IR -> Virtual, in loader.go)

I can't even yet figure out why there's that negindex thing.

Ye me neither, the dump unfortunately only include a position for IR and not virtual-opcode (as you can see in the GIST file).

We need to figure out why windows does a push32 0x8 at 0x7a instead of push push64
Windows also does a negindexu32  vs negindexu64.

It'd be helpful to somehow link 0x0007d to the respective IR, is that somehow feasible?

0x0007d		load64         0x0
is
0x00049	do_form_string:			; ..\cc\testdata\gcc-6.3.0\gcc\testsuite\gcc.c-torture\execute\20041214-1.c:38:7
0x0004a		variable        	&#1, **int8	; ..\cc\testdata\gcc-6.3.0\gcc\testsuite\gcc.c-torture\execute\20041214-1.c:39:2
0x0004b		argument        	&#2, **int8	; ..\cc\testdata\gcc-6.3.0\gcc\testsuite\gcc.c-torture\execute\20041214-1.c:39:29
0x0004c		dup             	**int8	; ..\cc\testdata\gcc-6.3.0\gcc\testsuite\gcc.c-torture\execute\20041214-1.c:39:29
0x0004d		load            	**int8	; ..\cc\testdata\gcc-6.3.0\gcc\testsuite\gcc.c-torture\execute\20041214-1.c:39:29
0x0004e		convert         	*int8, *int8	; ..\cc\testdata\gcc-6.3.0\gcc\testsuite\gcc.c-torture\execute\20041214-1.c:39:29
0x0004f		const           	0x8, uint32	; ..\cc\testdata\gcc-6.3.0\gcc\testsuite\gcc.c-torture\execute\20041214-1.c:39:11
0x00050		convert         	uint32, uint32	; ..\cc\testdata\gcc-6.3.0\gcc\testsuite\gcc.c-torture\execute\20041214-1.c:39:11
0x00051		element         	&[-uint32], *int8	; ..\cc\testdata\gcc-6.3.0\gcc\testsuite\gcc.c-torture\execute\20041214-1.c:39:29
0x00052		convert         	*int8, *int8	; ..\cc\testdata\gcc-6.3.0\gcc\testsuite\gcc.c-torture\execute\20041214-1.c:39:29
0x00053		store           	*int8	; ..\cc\testdata\gcc-6.3.0\gcc\testsuite\gcc.c-torture\execute\20041214-1.c:39:29
0x00054		convert         	*int8, **int8	; ..\cc\testdata\gcc-6.3.0\gcc\testsuite\gcc.c-torture\execute\20041214-1.c:39:11
0x00055		load<--here            	**int8	; ..\cc\testdata\gcc-6.3.0\gcc\testsuite\gcc.c-torture\execute\20041214-1.c:39:11

Yeah so it should be load64. Sounds like some code assumes sizeof(pointer)=long ?

Not sure if this helps you but the following line emits the push32 0x8:
https://github.com/cznic/virtual/blob/master/loader.go#L1481

Yeah so it should be load64.

It is load64. But the address computed on TOS is wrong. Don't yet understand why.

Sounds like some code assumes sizeof(pointer)=long ?

Seems like that, but looking everywhere I cannot find that.

Please try do change this in ccir/libc/builtin.h
from
#define __builtin_va_arg(ap, type) *(type*)(ap -= __roundup(sizeof(type), __SIZEOF_POINTER__))

to
#define __builtin_va_arg(ap, type) *(type*)(ap -= (__UINTPTR_TYPE__)(__roundup(sizeof(type), __SIZEOF_POINTER__)))

Kind of blind shot, Linux still works with it.

The IR assembly is higher-level though, right?

In a certain sense yes. It has type annotations. It avoids resolving offsets, which only the VM should know about, to constants numbers. Jumps targets are symbols, not addresses etc.

Yep that test now passes.
Now 20001203-2.c seems to fail with the same IR error as SQLite before.
Added the full trace to the GIST.

Yep that test now passes.
Now 20001203-2.c seems to fail with the same IR error as SQLite before.
Added the full trace to the GIST.

Okay, we have progress ;-)

Just so I'm not on the wrong track here:
# [55, err]: Verify (A): mismatched types, got int32, expected int64
		create_array_type:0x83: 	convert         	int64, *int8

that means that insteadof an int64 an int32 was passed as argument, right?
Also a stacktrace where that convert is emitted:
CONVERT:  {*int8 int64 ..\cc\testdata\gcc-6.3.0\gcc\testsuite\gcc.c-torture\execute\20001203-2.c:77:29}
goroutine 18 [running]:
runtime/debug.Stack(0xc042058008, 0xc042437890, 0x2)
        C:/Go/src/runtime/debug/stack.go:24 +0x80
runtime/debug.PrintStack()
        C:/Go/src/runtime/debug/stack.go:16 +0x29
github.com/cznic/ccir.(*c).convert(0xc04204c000, 0x8abca0, 0xc0423ed080, 0x8bb800, 0xc042395a40, 0x8bb800, 0xc0423451f0, 0x70c480, 0xc0423ca6c0)
        D:/projects/Go/gopath/src/github.com/cznic/ccir/ccir.go:1460 +0x2bd
github.com/cznic/ccir.(*c).expression(0xc04204c000, 0x8bb800, 0xc0423451f0, 0xc0423ed080, 0xc042395a40, 0x0)
        D:/projects/Go/gopath/src/github.com/cznic/ccir/ccir.go:1823 +0xd896
github.com/cznic/ccir.(*c).expression(0xc04204c000, 0x8bb800, 0xc0423aa7e0, 0xc0423ed440, 0x0, 0x0)
        D:/projects/Go/gopath/src/github.com/cznic/ccir/ccir.go:2087 +0x6843
github.com/cznic/ccir.(*c).expression(0xc04204c000, 0x0, 0x0, 0xc0423ed500, 0xc0423aa7e0, 0x4a)
        D:/projects/Go/gopath/src/github.com/cznic/ccir/ccir.go:2215 +0xb48f
github.com/cznic/ccir.(*c).expression(0xc04204c000, 0x8bb800, 0xc0423aa230, 0xc0423ed500, 0x8bb800, 0xc0423aa230)
        D:/projects/Go/gopath/src/github.com/cznic/ccir/ccir.go:1812 +0xd0aa
github.com/cznic/ccir.(*c).expressionList(0xc04204c000, 0x8bb800, 0xc0423aa230, 0xc042366f50, 0x8bb800, 0xc0423aa230)
        D:/projects/Go/gopath/src/github.com/cznic/ccir/ccir.go:2275 +0x85
github.com/cznic/ccir.(*c).expressionListOpt(0xc04204c000, 0x8bb800, 0xc0423aa230, 0xc0423645a0, 0x0)
        D:/projects/Go/gopath/src/github.com/cznic/ccir/ccir.go:2300 +0x11d
github.com/cznic/ccir.(*c).expressionStatement(0xc04204c000, 0xc0423ea100, 0x0)
        D:/projects/Go/gopath/src/github.com/cznic/ccir/ccir.go:2304 +0x68
github.com/cznic/ccir.(*c).statement(0xc04204c000, 0xc04243ce88, 0xc042351e00, 0x0)
.......

That means that insteadof an int64 an int32 was passed as argument, right?

The error comes from here
Not as an function argument but as the computed type of the TOS. Function argumens are loaded to the evaluation stack by the IR Argument opcode.

Also a stacktrace where that convert is emitted:

Yep, that proves ccir is wrongly thinking what's on the TOS. Fortunately, IR.Verify sees the error.
Compilers are complicated and this part (ccir) is only few month old. There must be a lot of similar bugs in it waiting to ruin everything ;-)
The line numbers are offset from what I see, please push your tree, thank you.

The line numbers are offset from what I see, please push your tree, thank you.
Done.

Ttherefore a convert int64 *int8 is emitted.
Then somehow in IR the type of that expresion is int32 !?
This mismatch (the reason of it is what we search) causes the error.

Meet our killer ;-)

Well nice!
That should be long long int for MINGW. A default GCC seems to return long int.
Any idea what the "correct" solution there would be?

This does not break Linux, please try if it fixes Windows:
jnml@4670:~/src/github.com/cznic/ccir$ git status
HEAD detached at bcbdb5a
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

	modified:   ccir.go

no changes added to commit (use "git add" and/or "git commit -a")
jnml@4670:~/src/github.com/cznic/ccir$ git diff
diff --git a/ccir.go b/ccir.go
index 49bc039..5f38a7b 100644
--- a/ccir.go
+++ b/ccir.go
@@ -1554,7 +1554,7 @@ func (c *c) binopType(n *cc.Expression) cc.Type {
                }

                if a.Kind() == cc.Ptr && b.Kind() == cc.Ptr && n.Case == 30 { // Expression '-' Expression                          // Case 30
-                       return c.ast.Model.LongType
+                       return n.Type
                }

                return t
jnml@4670:~/src/github.com/cznic/ccir$

Looks like that was the only direct reference to cc.Model.LongType in ccir/ccir.go, good for us.

Yep works (even the SQLite test runs into the next intruction trap VERY NICE! so the GetSystemTime() "syscall" works now)
C4 shows some breakage though:
--- FAIL: TestC4 (0.11s)
        all_test.go:959: [./c4]
                usage: c4 [-s] [-d] file ...
                4.501ms
        all_test.go:973: [./c4 hello.c]
                hello, world
                exit(0) cycle = 9
                2.4994ms
        all_test.go:977: [./c4 -s hello.c]
                1: #include <stdio.h>
                2:
                3: int main()
                4: {
                5:   printf("hello, world\n");
                    ENT  0
                    IMM  42992000
                    PSH
                    PRTF
                    ADJ  1
                6:   return 0;
                    IMM  0
                    LEV
                7: }
                    LEV
                2.5ms
        all_test.go:702: virtual.Exec: PANIC: runtime error: invalid memory address or nil pointer dereference
                testdata\github.com\rswier\c4\c4.c.main(0x2, 0x2880168)
                        testdata\github.com\rswier\c4\c4.c:501:1        0x004ff         load8          0x0      // else if (i == LC)  a = *(char *)a;                                    // load char
                D:\projects\Go\gopath\src\github.com\cznic\ccir\libc\crt0.c._start(0x3, 0x2880160)
                        D:\projects\Go\gopath\src\github.com\cznic\ccir\libc\crt0.c:15:1        0x0000e         callfp
// exit(((int (*)())main) (argc, argv));

Yep works (even the SQLite test runs into the next intruction trap VERY NICE! so the GetSystemTime() "syscall" works now)

:100:

C4 shows some breakage though:

C4 wants to be compiled in 32bit mode. I dirty-hacked it to work on both 32 and 64 bit Linux here. Because it has only one type, int, it now has a wrong pointer type (still int).
Try to change that line to read
#define int __PTRDIFF_TYPE__

Here that seems to work. (But I've not tested Linux/386 for some time.)

That leads to:
     all_test.go:959: [./c4]
                usage: c4 [-s] [-d] file ...
                2.0007ms
        all_test.go:970:
                got
                main() not defined
                exp
                hello, world
                exit(0) cycle = 9

NVM: need to change it at 2 places, fixed.

Ignore that test on Windows or delete it completely. It's not really valid C code after all. It was helpful at the early stages. Now it makes only troubles.

NVM: need to change it at 2 places, fixed.

Right, now I recall, once again in or somewhere before main(). Good job that you made it work on Windows as well!

Later on, please upload to gist the current TestSQLite output, it will make me happy just to look at it :stuck_out_tongue_winking_eye:

Yeah will do, first getting as many syscalls done as possible :)
We already get something like:
          Usage: ./test DATABASE SQL-STATEMENT
                5.4939ms
                []

Yeah will do, first getting as many syscalls done as possible :)

No worries.

We already get something like:

Tadaaa!

The offset-anonymous struct members transformation now also seems to work.
Fixed it after noticing it it was involved in a strange syscall failure...
ReadFile returned ERROR_SUCCESS since the _OVERLAPPED structure was wrongly
populated due to wrong offsets.

Well nearly:
all_test.go:1049: ["./test" "foo" "bar"]
                READ 0x56381, 0x56f07a8, 0x586fdd8, 100, 0FAIL (266) os_win.c:40417: (0) winRead(C:\Users\steff\AppData\Local\Temp\ccir-test-988818927\foo) - Der Vorgang wurde erfolgreich beendet.
                Can't open database: disk I/O error
                8.9981ms

So after the changes yesterday (fixing anonymous struct member offsets + sizeof) the SQLite Test results in the following:
unexpected fault address 0x340fff8
fatal error: fault
[signal 0xc0000005 code=0x0 addr=0x340fff8 pc=0x645835]

goroutine 18 [running]:
runtime.throw(0x75cbb0, 0x5)
        Go/src/runtime/panic.go:596 +0x9c fp=0xc05fba2130 sp=0xc05fba2110
runtime.sigpanic()
        Go/src/runtime/signal_windows.go:164 +0xff fp=0xc05fba2160 sp=0xc05fba2130
github.com/cznic/virtual.(*cpu).stackTrace(0xc04208cee0, 0x0, 0x0)
        /cznic/virtual/cpu.go:153 +0x631 fp=0xc05fba2378 sp=0xc05fba2160
github.com/cznic/virtual.(*cpu).run.func1(0xc04208cee0)
        /cznic/virtual/cpu.go:227 +0x1c5 fp=0xc05fba2448 sp=0xc05fba2378
runtime.call32(0x0, 0x775d48, 0xc0420485f0, 0x800000008)
        Go/src/runtime/asm_amd64.s:514 +0x4f fp=0xc05fba2478 sp=0xc05fba2448
panic(0x710020, 0x8e3bf0)
        Go/src/runtime/panic.go:489 +0x2dd fp=0xc05fba2510 sp=0xc05fba2478
runtime.panicmem()
        Go/src/runtime/panic.go:63 +0x65 fp=0xc05fba2530 sp=0xc05fba2510
runtime.sigpanic()
        Go/src/runtime/signal_windows.go:161 +0x8b fp=0xc05fba2560 sp=0xc05fba2530
github.com/cznic/virtual.(*cpu).run(0xc04208cee0, 0x2, 0x0, 0x0, 0x0)
        cznic/virtual/cpu.go:1365 +0x3e7a fp=0xc05fba3038 sp=0xc05fba2560
github.com/cznic/virtual.New(0xc04209e000, 0xc059414240, 0x3, 0x3, 0x8af020, 0xc0420a68c0, 0x8af0a0, 0xc0420a6930, 0x8af0a0, 0xc0420a69a0, ...)
        cznic/virtual/virtual.go:73 +0x2de fp=0xc05fba30f8 sp=0xc05fba3038
github.com/cznic/virtual.Exec(0xc04209e000, 0xc059414240, 0x3, 0x3, 0x8af020, 0xc0420a68c0, 0x8af0a0, 0xc0420a6930, 0x8af0a0, 0xc0420a69a0, ...)
        cznic/virtual/virtual.go:84 +0xf0 fp=0xc05fba31b0 sp=0xc05fba30f8
github.com/cznic/ccir.exec(0xc04204dad0, 0xc04209e000, 0xc059414240, 0x3, 0x3, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        cznic/ccir/all_test.go:714 +0x5f6 fp=0xc05fba34b0 sp=0xc05fba31b0
github.com/cznic/ccir.TestSqlite(0xc04204dad0)
        cznic/ccir/all_test.go:1051 +0xd89 fp=0xc05fba3fa8 sp=0xc05fba34b0
testing.tRunner(0xc04204dad0, 0x775c70)

Doing the following in cpu.go (L134):
	ip := c.ip - 1
	sp := c.sp
	ap := c.ap

	buff := DumpCodeStr(c.code, 0)
        fmt.Println(string(buff.Bytes())) // Content of the GIST

	for ip < uintptr(len(c.code)) {
		fmt.Printf("IP %x\n", ip) // prints only 0x9f6d

Huge IR (10MB) GIST.
Any idea what to look for there?

It looks like we may be not on the same commit. I see this cpu.go:153, but that seems innocent.
I haven't yet tested your CC changes, if there's some wrong offset computation it can crash the VM at many place, I guess. I should write a test or two so we can rule out that place as the source of the problem.
When I was solving similar troubles, I used the -tags virtual.trace to see the "black box" recordings. It calls this method and currently shows only line info. If you comment out (sorry no switch for that) these lines you should additionally get opcodes executed, stack pointer value and top 5 stack values. It may give some insight, please try it.
I think I'll be at home in ~one hour, I'll try to look more at the 10MB gist but it looks like we really need to add func and line info to the virtual code dump for it to be useful in this or any non-trivial size.

Wrt
0x09f60	func
0x09f61		push           ap
0x09f62		sub            sp, 0x8
0x09f63		push64         (ap-0x8)
0x09f64		load64         0x0
0x09f65		load64         0x10
0x09f66		argumentsfp
0x09f67		push64         (ap-0x8)
0x09f68		push64         (ap-0x10)
0x09f69		push32         (ap-0x18)
0x09f6a		push64         (ap-0x20)
0x09f6b		callfp
0x09f6c		store32
0x09f6d		return

0x09f6b looks like an indirect call of some int(*)(int64,int64,int32,int64), or s/int64/ptr/ anywhere.
0x09f60 looks like int foo(t* arg1, u* arg2) and the function pointer to later call at f6b seems to be obtained by doing some pointer arithtmetic (indexing) with the final load64 at f65 through as fnptr := *(sp+16), which looks like a 2nd item in a table listing function pointers.

Or f60 is maybe void(int *out, t* arg).

Seems like some imports were moved around (+ the added DumpCode):
It's https://github.com/cznic/virtual/blob/504ba1aa68141a67f3933a573a837a351f5cccc1/cpu.go#L150
Don't think commiting virtual makes sense yet since it will anyways break for linux (and
we don't really have other stacktrace points that are interesting?),
but let me know if you need those minor changes or if it's enough.

table listing function pointers

AFAIK sqlite uses that for syscalls and similar dispatch?
could be #define osCharUpperW ((LPWSTR(WINAPI*)(LPWSTR))aSyscall[2].pCurrent)
Currently dumping the new log using -tags virtual.trace with your suggested patches.
Going slow @4kb/s. Running for about a couple of minutes, reaching 3MB output size.
Will add it to the gist and post a link here once it's done...

*** Test killed: ran too long (10m0s).

welp, seems like this might quite a while.

Don't think commiting virtual makes sense yet since it will anyways break for linux (and
we don't really have other stacktrace points that are interesting?),
but let me know if you need those minor changes or if it's enough.

I'd like to Linux discover breakages early and continuously. I hope that way they can be fixed more easily - if it does not disrupt your workflow too much. Otherwise I can solve the breaks later, no problem.

AFAIK sqlite uses that for syscalls and similar dispatch?

Yes, SQLite uses FP tables in many places.

could be #define osCharUpperW ((LPWSTR(WINAPI*)(LPWSTR))aSyscall[2].pCurrent)

Could be something like that.

welp, seems like this might quite a while.

Probably not feasible at all time-wise.

Just having a couple of debug Printf's and stuff inbetween so i prefer to polish that bit atleast.
Generally no issue in doing that & commiting it, if that helps (which I doubt in this case?).
Currently at 2MB again, do you have some other proposition/need some additional input?
Else I think just letting it running and hoping it will be done eventually seems okay?

Need -timeout 24h or something. But the trace slows down the VM to maybe only few KIPS or even less. Meaning, if the crash does occur after after N million CPU cycles... :-(
Just today I created some profiles, you may want to look at them while you wait for your log ;-) Note: those are made with _profile_rate 1, so every single instruction is recorded and thus MIPS are very low. Normally I see up to 200 MIPS on Intel 4670 CPU.

Just today I created some profiles, you may want to look at them while you wait for your log ;-) Note: those are made with _profile_rate 1, so every single instruction is recorded and thus MIPS are very low. Normally I see up to 200 MIPS on Intel 4670 CPU.

Maybe I don't seem something here but those, while being pretty cool, don't actually help here, do they?
(Currently at 2MB again... I hope the end size is similar to the full-dump size so that would be 1/5)

Maybe I don't see something here but those, while being pretty cool, don't actually help here, do they?

They cannot help with ccir/ir/vm bugs. But those will be hunted down and meanwhile optimization will become important. So right, not useful yet except for having the record of the base performance now - for later comparison.

(Currently at 2MB again... I hope the end size is similar to the full-dump size so that would be 1/5)

I'm affraid that's a bit optimistic. The static code dump was ~400k instructions. The trace of every executed instruction can be several order of magnitude bigger. Really, if the crash does not occur early enough, if's not a feasible way to use the trace. It's a pitty the GCC tests did not catch anything. They are often short enough to trace fully.

Yeah it'd be really helpful to simply have a mapping from virtual bytecode -> IR
so we'd know where 0x09f6b comes from and what it's trying to do
(and then maybe detect a function argument mismatch or something).
How did you anyways debug more complex cases without that?
It might be just that the test cases cover enough functionality on linux,
while some typical windows-style just has less/none test-coverage,
and is therefore just not caught before... Quite unlucky but we'll figure it out.

Seems like I once also had luck, 11 MB :)
Didn't look at it yet but it's the same fault (cpu.go:153).
Maybe the IP 0x09f6b doesn't match anymore though.
Gist
Raw view of the GIST (better for this 11 MB file... since the UI is kinda slow)

Yeah it'd be really helpful to simply have a mapping from virtual bytecode -> IR (& source location)
so we'd know where 0x09f6b comes from and what it's trying to do
(and then maybe detect a function argument mismatch or something).

Agreed. Please push your tree, I'll do that first.

It might be just that the test cases cover enough functionality on linux,
while some typical windows-style just has less/none test-coverage,
and is therefore just not caught before... Quite unlucky but we'll figure it out.

Yep. Also, some of the quite new Windows "syscalls" ie. not VM assembly per se, can be the culprit. One of the last bugs before publishing sqlite was this.

Raw view of the GIST (better for this 11 MB file... since the UI is kinda slow)

Thanks. Will peek and if I cannot see something soon, will revert to implementing func/line info for VM dump.

Good, trace has line info.

Agreed. Please push your tree, I'll do that first.

Pushed virtual do you need anything else?

Pushed virtual do you need anything else?

No, thanks, I think that's enough.

If I interpret that output correctly it seems the last executed stuff is sqlite3.c:20141:1,
which is return id->pMethods->xRead(id, pBuf, amt, offset);
xRead is winRead (:40357) on windows and does a ReadFile (:40408), which is an implemented syscall.
So not seeing what goes wrong there.

If I interpret that output correctly it seems the last executed stuff is sqlite3.c:20141:1,
which is return id->pMethods->xRead(id, pBuf, amt, offset);

Agreed.

xRead is winRead (:40357) on windows and does a ReadFile (:40408), which is an implemented syscall.
So not seeing what goes wrong there.

But I don't see any #winRead or #readfile executed before the crash. Why?
jnml@4670:~/src/github.com/cznic/ccir$ grep -c '^0x00017' sqlite-fault-moreoutput.log
3
jnml@4670:~/src/github.com/cznic/ccir$

We are in the 3rd invocation of main, ie. the Third TestSqlite subtest.

This one
	all_test.go:1046: ["./test" "foo" "bar"]
		FAIL (1) near "bar": syntax error
		SQL error: near "bar": syntax error
		1.286687ms
		[0 foo]

It's the first one where the (0-sized) foo DB file is/should be created.

But I don't see any #winRead or #readfile executed before the crash. Why?

Seems like winRead is executed successfully (A printf before :40171 return winLogError(SQLITE_IOERR_READ, pFile->lastErrno,                        "winRead", pFile->zPath); is still outputed).
Maybe the return after somehow fails?

:20141 also seems to work:
 printf("Before");
  int ret = id->pMethods->xRead(id, pBuf, amt, offset);
  printf("After");

both are printed.

The very last lines:
	0x55bfc38: [10a 0 0 0 0]; ..\..\..\sqlite.org\sqlite-amalgamation-3180000\sqlite3.c:40416:1
0x09b76		store32        	0x55bfcc0: [10a 55bfd10 54407a8 55bfd30 55bfd60]; ..\..\..\sqlite.org\sqlite-amalgamation-3180000\sqlite3.c:20141:1
0x09b77		return
	0x55bfcc8: [10a 54407a8 55bfd30 55bfd60 2b75c]; ..\..\..\sqlite.org\sqlite-amalgamation-3180000\sqlite3.c:20141:1

sqlite3.c:40416:1 --> sqlite3.c:20141:1
so it seems like this return (:40416):
 return winLogError(SQLITE_IOERR_READ, pFile->lastErrno,"winRead", pFile->zPath);
fails.

At log line 99167 we are returning from test.c: errorLogCallback(), after printing an error message.
Other than that there is no other I/O operation in the trace of the 3d SQLite test. I must be missing something.
Syscall should be automaticall annotated by prefixing #. Does that work on Windows?

Syscall should be automaticall annotated by prefixing #. Does that work on Windows?

There should be UNSUPPORTEDOP when a windows-syscall is happening AFAIK
https://github.com/cznic/virtual/commit/2c4afdf874d1c760a821706428badb927c0a8056#diff-48e572644b37fcae7a61237628de2ee0

Yep, this needs to go away and you must list all new syscall here to see them in the trace, which is important for debugging.
Or just include op.Opcode in the 650 line, it has a .String() method for that. But please add also the # sign, it's handy in searching and grepping syscalls.
BTW, the tree does not build for me.
jnml@4670:~/src/github.com/cznic/virtual$ git status
On branch issue1
Your branch is up-to-date with 'origin/issue1'.
nothing to commit, working tree clean
jnml@4670:~/src/github.com/cznic/virtual$ git log -1
commit 2c4afdf874d1c760a821706428badb927c0a8056
Author: Steffen<EMAIL_ADDRESS>Date:   Tue Apr 25 19:50:45 2017 +0200

    current state [WIP, debug] will be rebased later...
jnml@4670:~/src/github.com/cznic/virtual$ go build
can't load package: package github.com/cznic/virtual:
windows_linux.go:3:1: expected 'package', found 'func'
jnml@4670:~/src/github.com/cznic/virtual$

Okay will do that & try to fix your build, new commit coming soon.

Simplest is to replace 'case' with 'default' here and delete the sycall list.
Anyway, I still see nothing so obviously wrong I could understand it. Have you put strace handling around your syscalls like here? If so, another tag you can use is -tags virtual.strace (no trace, only syscalls). It's much faster if it may show something.

I think I found it. Moment.

Pushed virtual (you may need to skip that commit if you already pulled, I overwrote the old one) again.

Here you start poping the arguments (sp increases), but IIUC, the last two are optional. This VM pushes left to right, if the number of arguments is non-constant you need to start from the argument pointer (ap) and go down. Example here.

Or the opt means just 'can be null'? :-(

Is HANDLE 64 bits?

HANDLE is void * so 8 bytes yes.

Or the opt means just 'can be null'? :-(

Honestly I'd assume that, but since you're asking I'm not quite sure.
Doing some digging...

https://blogs.msdn.microsoft.com/oldnewthing/20161026-00/?p=94585

The opt part means that the parameter is optional (may be null).

Pushed virtual (you may need to skip that commit if you already pulled, I overwrote the old one) again.

Ok, I see nothing wrong.

The opt part means that the parameter is optional (may be null).

Sorry for the false alarm. I was so happy for a minute.

I'll start a test&trace run again and probably have a log file in ~20 minutes.

fn/line info added: https://github.com/cznic/virtual/commit/ceb61f7a9799a12b59640dd27a01295cfb031c82
You need also https://github.com/cznic/ccir/commit/6b3a2e8039ce9c474ccfe2a74f8651044743043b

7/11MB. I guess I wait this out, post the log and try to get a bytecode dump and hope the bytecode<->line mapping works. Then we can correlate both logs and see if we know more.
Remarks?

You can test drive it
$ go test -v -run TCC -re 00_ -log |& tee log

Sorry, don't know how's the command written on Windows.

Works for TCC, just have to figure out how to do it for Sqlite.
(Since it's a fault I have to inject the DumpCodeString again and first have to figure out how that now works, reading your commit diff)

Figured it out line trace: https://gist.github.com/steffengy/388a129e5daf90f1cf61ad1b41e80dbd/raw/fc446b3a3795a748b304a90915095f97fe569efb/line-trace.log
Other log at 10/11 MB.

Just replace TCC with SQLite and remove the -re 00_. Nothing else is needed IMO, assuming you add -tags virtual.trace.
I strongly suggest to handle "stracing" so we can see a human readable verison of syscall arguments. If there's something wrong with them it may catch our attention. Use GoString() where the arg is a null terminated string.

Github gists download really slow.

30% cca now.

Yeah sorry 34MB unfortunately, works quite okay for me though.
AFAIK IP should actually be 0x09b78, since 0x09b77 is the last executed OPCode.
(ip := c.ip - 1 in cpu.go)
Doesn't change much though, same line.

I strongly suggest to handle "stracing" so we can see a human readable verison of syscall arguments.

I'll add it and uploaded the strace output.
(Fortunately we likely won't need those slow logs anymore, will still add the 11MB file shortly, when it's finally done)

Yeah sorry 34MB unfortunately, works quite okay for me though.

Not your fault. I don't know if it's Github or Vodafone's, but it look like 100k[b|B]ps.
Since I upgraded to VDSL I pay twice for the original speed most of the time, lol.

(Fortunately we likely won't need those slow logs anymore, will still add the 11MB file shortly, when it's finally done)

There's hope ;-)

Looking at the line log L405842 and before, it looks like a perfectly fine and corerct termination until the last return. So corrupted stack likely.

Okay, just for completeness (and since i waited long enough for it):
https://gist.githubusercontent.com/steffengy/388a129e5daf90f1cf61ad1b41e80dbd/raw/9d32683fdff3f0fc8db5dbb9db528bbf99848978/sqlite-fault-moreoutput.log

Looking at the line log L405842 and before, it looks like a perfectly fine and corerct termination until the last return. So corrupted stack likely.

Err, that's not trace, just dump.

There's exactly one #readfile at L90057. Input args I dont know, but stack after return at L90061 looks corrupted suspiciously to me.

I take that back.

GetCurrentThreadId()
LeaveCriticalSection(&{0 0 1 1 0 0})
ReadFile(64, 89193944, 100, 89193560, 89193576) 0 Ende der Datei (EOF) erreicht.
GetLastError() 0 Der Vorgang wurde erfolgreich beendet.
GetLastError() 0 Der Vorgang wurde erfolgreich beendet.
CloseHandle(4864, 0, 0, 0, 89192864, 0, 0) 40 Der Vorgang wurde erfolgreich beendet.
WideCharToMultiByte(65001, 0, 11201008, -1, 0, 0, 0, 0) 41 Der Vorgang wurde erfolgreich beendet.
GetCurrentThreadId()
EnterCriticalSection(&{0 0 0 0 0 0})
GetCurrentThreadId()
GetCurrentThreadId()
GetCurrentThreadId()
GetCurrentThreadId()
GetCurrentThreadId()
LeaveCriticalSection(&{0 0 1 1 0 0})
WideCharToMultiByte(65001, 0, 11201008, -1, 87625584, 41, 0, 0) 41 Der Vorgang wurde erfolgreich beendet.
LocalFree(11201008) 0 Der Vorgang wurde erfolgreich beendet.
GetCurrentThreadId()
EnterCriticalSection(&{0 0 0 0 0 0})
GetCurrentThreadId()
GetCurrentThreadId()
GetCurrentThreadId()
LeaveCriticalSection(&{0 0 1 1 0 0})
ATTEMPTING 9b77

STRACE is also pushed (so you can check the arguments etc.)
Some german translation (since go translates these errors into the current locale):
"Der Vorgang wurde erfolgreich beendet." just means "operation successfully completed".

If I'm not missing some syscall a round 9b77 it seems it might be unrelated to them?
Or I'm missing something...

Thanks for handling strace. Please use the %#x format for arguments, or at least for pointer types.

ReadFile(64, 89193944, 100, 89193560, 89193576) 0 Ende der Datei (EOF) erreicht.

Where is some preceeding file open and why is the file handle, a pointer as I learnt today, 64? Other arguments look fine to me, I guess.

If I'm not missing some syscall a round 9b77 it seems it might be unrelated to them?
Or I'm missing something...

Have yet to look where's tha ATTEMPTING msg printed...

I see it.
I think cpu.go/stackTrace() is broken. It should not crash even for corrupted stack but somehow cleanly exit. How to detect a valid address is not clear to me however. Or I have to wrap reads in defer/recover and handle the fault, which I'm not sure is even always possible b/c sometime I see 'nil derefence' and sometimes different messages.

Please use the %#x format for arguments,
Done.

Have yet to look where's tha ATTEMPTING msg printed...
https://github.com/cznic/virtual/blob/52f92746b52bf50e36204f5a4d4a0ad1c18376bd/cpu.go#L142

64? Other arguments look fine to me, I guess.
Yeah I agree that's a bit strange. I'll look into this.

Full STRACE Log

Where is some preceeding file open
GOOD question.

64? Other arguments look fine to me, I guess.

That value is valid. HANDLE just specifies something that the kernel maps using the process
to an "internal" object. void * is probably just used because it's guaranteed to be 8 bytes on
windows. (Reproduced using a C script compiled with MINGW and also got 124 for a test file)