When attempting to debug #3162 I had noticed spurious behaviour around
what I assumed to be eflags getting corrupt around inlined syscalls.
This turned out to be a red herring but to ensure we are still testing
this, create a fully fleshed out unit test.
This test ensures a couple of things.
1) A flag that is set or unset before a syscall doesn't have its data
corrupt
2) An inline syscall doesn't corrupt the eflags, checking the eflag
result after returning from the syscall.
3) A signal occuring while in an inline syscall returns the correct
eflags information in the signal handler information
This test gets accomplished by setting or unsetting a particular flag
and then calling the futex syscall in a way that is guaranteed to be
inlined and also wait forever. Then the parent thread will signal with a
SIGTERM and read back the signal information. It does this multiple
times for each flag we care about.
While tracking issues in #3162, I had encountered a random crash that I
started hunting. It was very quickly apparent that this crash was
unrelated to that PR. I just happened to be running a unittest that was
creating and tearing down a bunch of threads that exacerbated the
problem.
See as follows with the strace output:
```
[pid 269497] munmap(0x7fffde1ff000, 16777216) = 0
[pid 269497] munmap(0x7fffde1ff000, 16777216 <unfinished ...>
[pid 268982] mmap(NULL, 16777216, PROT_READ|PROT_WRITE|PROT_EXEC, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fffde1ff000
[pid 269497] <... munmap resumed>) = 0
```
One thread is freeing some memory with munmap, another one then does a mmap and gets the same address back.
Nothing too crazy at initial glance, but taking a closelier look, we can
see that there are two strange oddities:
1) We are double unmapping the same address range through munmap
2) The second munmap is interrupted and returns AFTER the mmap.
This has the unfortunate side-effect that the mmap that just returned
the same address has actually just been unmapped! This was resulting in
spurious crashes around thread creation that was SUPER hard to nail
down.
The problem comes down to how code buffer objects are managed, in
particular how the Arm64Emitter and Dispatcher handled its buffers.
Arm64Emitter is inherited by two classes; Dispatcher, and Arm64JITCore.
On class destruction the emitter would free its internal tracking
buffer. Additionally on destruction, the Arm64JITCore would walk through
all of its CodeBuffers and free them. The problem ends up being that in
the Arm64JITCore, it would free its code buffers which also ended up
being the current active buffer bound to the Arm64Emitter. Thus causing
the Arm64Emitter to come back around and try to free the same buffer
again.
This is a double-free problem! and was only visible on thread exiting!
Can't track double frees with mmap and munmap with current tooling!
This problem typically didn't occur because of how fast the destruction
usually takes and jemalloc inbetween also typically means the problem
doesn't occur. Initially thinking this was a threaded pool allocator bug
because typically the new allocation would end up in there once a new
thread was spinning up.
Now we change behaviour, Arm64Emitter doesn't do any buffer management
itself, instead just passing an initial buffer on to its internal buffer
tracking if given one up front.
This leaves the Dispatcher and the Arm64JITCore to do their buffer
management and ensuring there is no double free.
The day is saved!
This mask was being used incorrectly, it's a GPR spill mask for host
GPRs not an index in to the SRA array. Search the array of SRA registers
for the first one in the mask first to use as a temporary.
Fixes an issue with 32-bit inline syscalls where the first register
being spilled was r8, which was beyond the size of SRA registers on
32-bit processes. This would cause FEX to read the value just after
x32::SRA which is x32::RA. This would mean it would use r20 as a
temporary, corrupting the register in the process.
I noticed this while poking at #3162, but also when I was looking at a
memory buffer ownership problem.
Requires #3249 to be merged first
Library alerting has been disabled for now, and storing IR while
gdbserver is running is removed.
Otherwise no functional change.
When attempting to read files that aren't backed by a filesystem then
our current read file helpers fail since they query the file size
upfront.
Change the helper so that it doesn't query the size and just reads the file if it
can be opened. This lets us read `/proc/self/maps` using helpers.
GDBServer is inherently OS specific which is why all this code is
removed when compiling for mingw/win32. This should get moved to the
frontend before we start landing more work to clean this interface up.
Not really any functional change.
Changes:
FEXCore/Context: Adds new public interfaces, these were previously
private.
- WaitForIdle
- If `Pause` was called or the process is shutting down then this
will wait until all threads have paused or exited.
- WaitForThreadsToRun
- If `Pause` was previously called and then `Run` was called to get
them running again, this waits until all the threads have come out
of idle to avoid races.
- GetThreads
- Returns the `InternalThreadData` for all the current threads.
- GDBServer needs to know all the internal thread data state when the
threads are paused which is what this gives it.
GDBServer:
- Removes usages of internal data structures where possible.
- This gets it clean enough that moving it out of FEXCore is now
possible.
These should always be used in the dispatcher rather than the raw jumps they
translate to, as they ensure that flags are flushed. Eliminates a class of bugs
that will become a lot easier to hit with the new nzcv work.
Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Requires #3238 to be merged first since this uses the tbx IR operation.
Worst case is now a three instruction sequence of ldr+ldr+tbx.
Some operations are special-cased, which definitely doesn't cover all
possible cases we could use without tbx, but as a worst case improvement
this is a significant improvement.
We can't sanely test all 256 swizzle masks, so walk the array of random
data for each swizzle and CRC the results each step along the way.
This will give us a final result rather than each individual step.
A bunch of blendps swizzles weren't optimal. This optimizes all swizzles
to be optimal.
Two instructions can be more optimal without a tbx but the rest required
tbx to be optimal since they don't match ARM's swizzle mechanics.
gdbserver is currently entirely broken so this doesn't change behaviour.
The gdb pause check that we originally had an excessive amount of
overhead.
Instead use the pending interrupt fault check that was wired
up for wine.
This makes the check very lightweight and makes it more reasonable to
implement a way to have gdbserver support attaching to a process.
gdb gets angry if we return text with `<No Name>` in an xml text field.
Instead only return a name if we have one and gdb will take care of the
rest.
Additionally change the formatting of the return packet, it doesn't need
the xml version header.
Messed up when originally implementing this, substr's second argument is
requested substring length, not the ending position.
Noticed this while trying to parse multiple FEX_HOSTFEATURES options.
Ryan Houdek