-
Notifications
You must be signed in to change notification settings - Fork 17.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
runtime: access violation during cgo call in github.com/mattn/go-sqlite3 on windows 7 64-bit #9356
Comments
Your repro does not check for a possible error on Open
|
You are right. The original repro did check the error and it was nil. I removed all the printf goo i had in there right before submitting the report. As you can see from the stack trace, Close() is being called on a valid pointer. I will update the repro instructions. |
It's not reproduced on windows 32bit OS at least. |
I have some more information. I rolled back the version of github.com/mattn/go-sqlite3 to a previous version where the only thing that changed is the amalgamated sqlite3 code. The stars aligned such that the bug was not hit. I instrumented the code in sqlite3 so i could dump the aSyscall array and observe the function pointers it was using for the different kernel32 functions. When everything is working, every function pointer in the aSyscall array properly points to the import thunks for the kernel32 functions:
The dissassembly for the import thunks:
Then, i changed the repro code to add a cgo function that uses a few kernel32 functions. The cgo function is not called by repro code. Just having it there causes the function pointer in the aSyscall table for that function to not be an address to an import thunk, but rather the address to the Import Address Table entry for the function. New repro code:
aSyscall dump of broken version:
and the import thunks:
In this instance, import thunks for CloseHandle, CreateMutexW, and FlushFileBuffers are not even generated. The aSyscall entries for these functions are set to the address of the Import Address Table entries for the functions. For example, kernel32!CreateMutexW resides at 00000000
The function pointer for CreateMutexW in the aSyscall array should be to an import thunk that looks something like the following:
Instead, the function pointer is 00000000`00644058, which is wrong. It should either set to an import thunk which jumps to the address stored in the IAT for kernel!CreateMutexW or the direct address to kernel32!CreateMutexW. Long story short, when the function is called, the addresses in the IAT are executed as code, beginning at 00000000`00644058, which of course crashes. Here is my function to dump the aSyscall table:
And i call it in the sqlite3.go init function:
|
Hmm, as far as i can see, changes of amalgamated code seems not effect to this issue. |
i don't see changes in logic. the changes which may effect seems to be only function table. mattn/go-sqlite3@d369cbb#diff-ccfdd787473b0eb7a494298ebdb3b35dL34 others are not change the behavior, i guess. |
I'm not fully certain, but I'm pretty sure it isn't sqlite's fault, other than it happens to trigger the bug somewhere in the toolchain. If I "go install" github.com/mattn/go-sqlite3 on ref beeda4c, everything works fine, but if I add some cgo calls into other kernel32 functions in my code, then it breaks. Since the compiler already generated the code (i.e. go-sqlite.a) and the only thing changing is my code and then the linker putting it all into a PE file, I'm pretty sure it is something in the linker. Its as if it knows it should initialize the entry for the kernel32 function in the aSyscall array with an address to a generated import thunk (i.e. code that does a "jmp [X]"), but instead initializes it with X (i.e. the address into the IAT storing the real address of the kernel32 function). I started digging into the various sections in the go-sqlite3.a, trying to find asyscall to see how it was getting statically initialized, but I haven't had the time to dig deeper. |
Ah, I reproduced it just now on windows 32bit OS. |
GitHub diff view seems strange. Below is a whole diff between previous and current. |
And seems go crashed at sqlite3BtreeLeave(). I'm thinking this is a bug of amalgamated code not go or driver. I'l check diff and will revert amalgamated code. |
I don't mind you keep figure-out a cause of bug. :) |
Please check this change https://go-review.googlesource.com/5711 to see if it fixes your problem. Alex |
I'll try this. |
Seems not fixed.
|
@mattn it works for me. I applied my change with "git fetch https://go.googlesource.com/go refs/changes/11/5711/1 && git checkout FETCH_HEAD" command. Built Go. I use gcc (GCC) 4.9.1. I use beeda4c31194f4cdb15eb89d2f2b4be4474371c4 version of github.com/mattn/go-sqlite3. I use this http://play.golang.org/p/8govbvHdB2 test program (as per #9356 (comment)). This is what test program prints:
But if I change to current Go tip (9b3ccc0). Same program prints:
(make sure you re-install github.com/mattn/go-sqlite3). Please, provide instructions for me to reproduce what you see there. Thank you. Alex |
@denji if you talking to me, I don't understand what you are saying. Alex |
@alexbrainman did you test with mattn/go-sqlite3@d369cbb ? |
I didn't test against mattn/go-sqlite3@d369cbb. Should I? I thought we had conversation about beeda4c31194f4cdb15eb89d2f2b4be4474371c4 all along. You don't expect me to debug / test every change of mattn/go-sqlite3. Do you? Perhaps you should see what has changed between these two version, and decide for yourself what to do. |
As I said in above, I reverted the commit. This behavior doesn't be reproduced with older amalgamated code. mattn/go-sqlite3@d369cbb is commit that be possible to reproduce on my environment. |
I tested mattn/go-sqlite3@d369cbb now. It works fine as I have described up above. Alex |
I could reproduce this on Win7 32bit
|
Running http://play.golang.org/p/8govbvHdB2 against d369cbb does not crashes for me on windows-386 even without my change. I use gcc (GCC) 4.8.1 there. Alex |
Here are the problems that I know of in GCC 4.8/4.9.x:
Can solve the problem or search dereferencing pointer in the code. |
I'm using gcc 4.8.2, so added |
This changeset fixes the bug with my original repro configuration. |
Should be fixed by external linking support, #4069. |
The test is a simple reproduction of issue 9356. Update #8948. Update #9356. Change-Id: Ia77bc36d12ed0c3c4a8b1214cade8be181c9ad55 Reviewed-on: https://go-review.googlesource.com/7618 Reviewed-by: Minux Ma <minux@golang.org>
CL https://golang.org/cl/5711 mentions this issue. |
So this didn't make it into Go1.5 and the CL references 1.6 but doesn't seem to have been merged. Anybody know the status of this? |
Somebody needs to review the CL or write a new one. Alex and I were unable to come to agreement on it. |
What @minux said. go1.5 uses external linker (mingw) to build cgo programs, so you should not see your problem now. The broken code in Go linker still remain, but it is not used any more. I hope it explains the situation. Alex |
OS: Windows 7 64-bit
GCC version: gcc (x86_64-posix-seh-rev1, Built by MinGW-W64 project) 4.9.1
Go version: 1.4 (but also happens on 1.2+, didn't check < 1.2)
Description:
when the below program is compiled and run, the go program either panics or crashes outright.
We did some spelunking and from what we can tell, the crash happens when sqlite3 tries to call a kernel32.dll function from its aSyscall array. The function pointer is supposed to point to an import jump table, but instead points to a page close to the jump table that has some garbage in it. Sampling other function pointers in the aSyscall array show they properly go the the import jump table.
We've used github.com/mattn/go-sqlite3 for quite a while with no issues. We made some unrelated changes and recompiled our application and suddenly hit the issue. Recompiling the program without the changes stopped triggering the bug, but once we found the issue we've been able to hit it with smaller programs (see the program below)
We suspect a linker issue, which is why we reported it here.
Repro instructions:
An example of the output:
The text was updated successfully, but these errors were encountered: