This was the result of a typo accidentally introduced in
e51d715700. This restores the previous
correct behavior.
The behavior with the reference was incorrect and would cause some games
to fail to boot.
Conceptually, it doesn't make sense for a thread to be able to persist
the lifetime of a scheduler. A scheduler should be taking care of the
threads; the threads should not be taking care of the scheduler.
If the threads outlive the scheduler (or we simply don't actually
terminate/shutdown the threads), then it should be considered a bug
that we need to fix.
Attributing this to balika011, as they opened #1317 to attempt to fix
this in a similar way, but my refactoring of the kernel code caused
quite a few conflicts.
Specifically bugs/crashes that arise when putting them in positions that are legal but not typical, such as midline, between patch data, or between patch records.
Many of the member variables of the thread class aren't even used
outside of the class itself, so there's no need to make those variables
public. This change follows in the steps of the previous changes that
made other kernel types' members private.
The main motivation behind this is that the Thread class will likely
change in the future as emulation becomes more accurate, and letting
random bits of the emulator access data members of the Thread class
directly makes it a pain to shuffle around and/or modify internals.
Having all data members public like this also makes it difficult to
reason about certain bits of behavior without first verifying what parts
of the core actually use them.
Everything being public also generally follows the tendency for changes
to be introduced in completely different translation units that would
otherwise be better introduced as an addition to the Thread class'
public interface.
GetName() returns a std::string by value, not by reference, so after the
std::string_view is constructed, it's not well defined to actually
execute any member functions of std::string_view that attempt to access
the data, as the std::string has already been destroyed. Instead, we can
just use a std::string and erase the last four characters.
When searching for a file extension, it's generally preferable to begin
the search at the end of the string rather than the beginning, as the
whole string isn't going to be walked just to check for something at the
end of it.
If an error occurs when constructing the PartitionFilesystem instance,
the constructor would be exited early, which wouldn't initialize the
extracted data member, making it possible for other code to perform an
uninitialized read by calling the public IsExtractedType() member
function. This prevents that.
Like the other two bits of factored out code, this can also be put
within its own function. We can also modify the code so that it accepts
a const reference to a std::vector of files, this way, we can
deduplicate the file retrieval.
Now the constructor for NSP isn't a combination of multiple behaviors in
one spot. It's nice and separate.
This too, is completely separate behavior from what is in the
constructor, so we can move this to its own isolated function to keep
everything self-contained.
If any of the error paths before the NCA retrieval are taken, it'll
result in program_nca_status being left in an inconsistent state. So we
initialize it by default with a value indicating an error.
In some games (Splatoon 2 and Splatoon 2 Splatfest World Premiere, notably), pass offset=0 and count=2047 into the ListAddOnContent method which should return all DLCs for the current title. The (presumably) intended behavior is to successfully return a empty array but because of a < v. <= in an if statement, a failure error code was returned causing these games to svcBreak. This fixes that if statement.
Now that we have all of the rearranging and proper structure sizes in
place, it's fairly trivial to implement svcGetThreadContext(). In the
64-bit case we can more or less just write out the context as is, minus
some minor value sanitizing. In the 32-bit case we'll need to clear out
the registers that wouldn't normally be accessible from a 32-bit
AArch32 exectuable (or process).
This will be necessary for the implementation of svcGetThreadContext(),
as the kernel checks whether or not the process that owns the thread
that has it context being retrieved is a 64-bit or 32-bit process.
If the process is 32-bit, then the upper 15 general-purpose registers
and upper 16 vector registers are cleared to zero (as AArch32 only has
15 GPRs and 16 128-bit vector registers. not 31 general-purpose
registers and 32 128-bit vector registers like AArch64).
Makes the public interface consistent in terms of how accesses are done
on a process object. It also makes it slightly nicer to reason about the
logic of the process class, as we don't want to expose everything to
external code.
Internally within the kernel, it also includes a member variable for the
floating-point status register, and TPIDR, so we should do the same here to match
it.
While we're at it, also fix up the size of the struct and add a static
assertion to ensure it always stays the correct size.
A process should never require being reference counted in this
situation. If the handle to a process is freed before this function is
called, it's definitely a bug with our lifetime management, so we can
put the requirement in place for the API that the process must be a
valid instance.
boost::static_pointer_cast for boost::intrusive_ptr (what SharedPtr is),
takes its parameter by const reference. Given that, it means that this
std::move doesn't actually do anything other than obscure what the
function's actual behavior is, so we can remove this. To clarify, this
would only do something if the parameter was either taking its argument
by value, by non-const ref, or by rvalue-reference.
The std::vector instances are already initially allocated with all
entries having these values, there's no need to loop through and fill
them with it again when they aren't modified.
auto x = 0;
auto-deduces x to be an int. This is undesirable when working with
unsigned values. It also causes sign conversion warnings. Instead, we
can make it a proper unsigned value with the correct width that the
following expressions operate on.
Given these are only added to the class to allow those functions to
access the private constructor, it's a better approach to just make them
static functions in the interface, to make the dependency explicit.
This converts it into a regular constructor parameter. There's no need
to make this a template parameter on the class when it functions
perfectly well as a constructor argument.
This also reduces the amount of code bloat produced by the compiler, as
it doesn't need to generate the same code for multiple different
instantiations of the same class type, but with a different fill value.
The locations of these can actually vary depending on the address space
layout, so we shouldn't be using these when determining where to map
memory or be using them as offsets for calculations. This keeps all the
memory ranges flexible and malleable based off of the virtual memory
manager instance state.
