|
05:05
japhb left
05:09
japhb joined
|
|||
| timo | patrickb: struct termios has a sizeof of 72 on mac, and 60 on linux, so that causes memory corruption on mac when Terminal::MakeRaw is used by Repl | 09:41 | |
| via Terminal::LineEditor | |||
| 0x38 is the offsetof c_ospeed on linux, 0x40 on mac, offsetof c_cc has 0x11 on linux and 0x20 on mac | 09:44 | ||
| iflag, oflag, cflag, and lflag are at hex 0, 8, 10, 18 on mac, but on linux they are 0, 4, 8, and c | 09:47 | ||
| typedef unsigned long tcflag_t on mac, but on linux i get sizeof(tcflag_t) giving 4 | 09:49 | ||
| yeah, unsigned int on linux | |||
| lizmat | *phew* nice! | 11:09 | |
| timo | if all we do is call cfmakeraw on the termios we get, we don't have to get the fields right, all we need to do is ensure the struct is big enough in total so the native functions that act on the termios struct don't write past what we have allocated | 11:44 | |
| otherwise we can determine the size of the struct by looking at which interpretation gives reasonable values when we just try both :D | 11:52 | ||
| maybe by initialising the struct with nonsense values, calling tcgetattr, and then seeing how much of the values have been overwritten with anything | |||
| lizmat: could you try a locally changed version of Terminal::MakeRaw that uses the struct definition that's on macos? here's how it's defined on go for example: cs.opensource.google/go/x/sys/+/ma...4.go;l=613 compared to the linux amd64 version cs.opensource.google/go/x/sys/+/ma...4.go;l=285 which also has that Line field that the other | 11:57 | ||
| doesn't have, but 19 instead of 20 flag byte fields | |||
| or i guess in order to make it not crash, just add like ten dummy 64 bit integer fields at the end of the struct just so the allocation we do gets biggered | |||
| lizmat will ry | 11:58 | ||
| try | |||
| timo | i'm a bit frustrated that it wasn't the first thing i checked, just by intuition, when i started diagnosing this | 12:00 | |
| lot of time wasted :( | |||
| lizmat | meh, sorry, I didn't realize it could be something that obvious | 12:01 | |
| timo | haha | ||
| no problem | |||
| i do not understand why asan didn't catch the obvious issue though | |||
| i'll try later today with a struct that is completely too small and see if that changes things | |||
| lizmat | so how did you get to 72 ? | 12:02 | |
| timo | i was in lldb and did `print sizeof(struct termios)` | ||
| lizmat | I see 4 * 8 + 20 + 2 * 8 ? | ||
| timo | check your local xcode sdk for a sys/termios.h, too | ||
| there's probably padding too | |||
| lizmat | ah | 12:03 | |
| timo | see the offsetof results for c_ospeed | ||
| 0x38 on linux, 0x40 on mac, that would be 4 * upgrade from 4 to 8 byte and then some more i guess? | |||
| lizmat | yeah, so ispeed and ospeed are int32 on Linux and int64 on MacOS | ||
| timo | and the four flag members at the start | 12:04 | |
| lizmat | perhaps an approach such as here: github.com/lizmat/P5getpwnam/blob/...am.rakumod | 12:06 | |
| would be more appropriate ? | |||
| aka, different classes for different OSes | |||
| timo | sure | ||
| lizmat | with a selection at compile time: github.com/lizmat/P5getpwnam/blob/...akumod#L66 | 12:07 | |
| timo | AFK | 12:08 | |
| lizmat | yeah, adding: | 12:33 | |
| + has int64 $.filler1; | |||
| + has int32 $.filler2; | |||
| to the struct fixes the issue | |||
| patrickb | Can we just hard code the different struct layouts? | 12:36 | |
| We'll probably want to access other parts of the structure in the future. | |||
| That's what go is doing, right? | 12:37 | ||
| Ah, lizmat proposed the same. | |||
| lizmat | yeah, yup :-) | 12:38 | |
| japhb | timo++, lizmat++ | 17:07 | |