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.
Previously, these were reporting hardcoded values, but given the regions
can change depending on the requested address spaces, these need to
report the values that the memory manager contains.
Rather than hard-code the address range to be 36-bit, we can derive the
parameters from supplied NPDM metadata if the supplied exectuable
supports it. This is the bare minimum necessary for this to be possible.
The following commits will rework the memory code further to adjust to
this.
* Implemented fatal:u properly
fatal:u now is properly implemented with all the ipc cmds. Error reports/Crash reports are also now implemented for fatal:u. Crash reports save to yuzu/logs/crash_reports/
The register dump is currently known as sysmodules send all zeros. If there are any non zero values for the "registers" or the unknown values, let me know!
* Fatal:U fixups
* Made fatal:u execution break more clear
* Fatal fixups
* Stubbed IRS
Currently we have no ideal way of implementing IRS. For the time being we should have the functions stubbed until we come up with a way to emulate IRS properly.
* Added IRS to logging backend
* Forward declared shared memory for irs
Preserves the meaning/type-safetiness of the stream state instead of
making it an opaque u32. This makes it usable for other things outside
of the service HLE context.
Even though setting this value to 3 is more correct. We break more games than we fix due to missing implementations. We should keep this as 0 for the time being
The owning process of a thread is required to exist before the thread,
so we can enforce this API-wise by using a reference. We can also avoid
the reliance on the system instance by using that parameter to access
the page table that needs to be set.
Several classes have a lot of non-trivial members within them, or don't
but likely should have the destructor defaulted in the cpp file for
future-proofing/being more friendly to forward declarations.
Leaving the destructor unspecified allows the compiler to inline the
destruction code all over the place, which is generally undesirable from
a code bloat perspective.
This was used in two different translation units
(deconstructed_rom_directory and patch_manager). This means we'd be
pointlessly duplicating the whole array twice due to it being defined
within the header.
These variables aren't used, which still has an impact, as std::vector
cannot be optimized away by the compiler (it's constructor and
destructor are both non-trivial), so this was just wasting memory.
std::shared_ptr isn't strictly necessary here and is only ever used in
contexts where the object doesn't depend on being shared. This also
makes the interface more flexible, as it's possible to create a
std::shared_ptr from a std::unique_ptr (std::shared_ptr has a
constructor that accepts a std::unique_ptr), but not the other way
around.
An instance of the NAX apploader already has an existing NAX instance in
memory. Calling directly into IdentifyType() directly would re-parse the
whole file again into yet another NAX instance, only to toss it away
again.
This gets rid of unnecessary/redundant file parsing and allocations.
AsNCA() allocates an NCA instance every time it's called. In the current
manner it's used, it's quite inefficient as it's making a redundant
allocation.
We can just amend the order of the conditionals to make it easier to
just call it once.
* Reworked incorrect nifm stubs
Need confirmation on `CreateTemporaryNetworkProfile`, unsure which game uses it but according to reversing. It should return a uuid which we currently don't do.
Any 0 client id is considered an invalid client id.
GetRequestState 0 is considered invalid.
* Fixups for nifm
* Fix bug where default username value for yuzu_cmd create an userprofile with uninitialize data as username
* Fix format
* Apply code review changes
* Remove nullptr check
This can just be a regular function, getting rid of the need to also
explicitly undef the define at the end of the file. Given FuncReturn()
was already converted into a function, it's #undef can also be removed.
Previously the second half of the value being written would overwrite
the first half. Thankfully this wasn't a bug that was being encountered,
as the function is currently unused.
This modifies the CPU interface to more accurately match an
AArch64-supporting CPU as opposed to an ARM11 one. Two of the methods
don't even make sense to keep around for this interface, as Adv Simd is
used, rather than the VFP in the primary execution state. This is
essentially a modernization change that should have occurred from the
get-go.
The kernel does the equivalent of the following check before proceeding:
if (address + 0x8000000000 < 0x7FFFE00000) {
return ERR_INVALID_MEMORY_STATE;
}
which is essentially what our IsKernelVirtualAddress() function does. So
we should also be checking for this.
The kernel also checks if the given input addresses are 4-byte aligned,
however our Mutex::TryAcquire() and Mutex::Release() functions already
handle this, so we don't need to add code for this case.
Courtesy of @ogniK5377.
This also moves them into the cpp file and limits the visibility to
where they're directly used. It also gets rid of unused or duplicate
error codes.
The kernel caps the size limit of shared memory to 8589930496 bytes (or
(1GB - 512 bytes) * 8), so approximately 8GB, where every GB has a 512
byte sector taken off of it.
It also ensures the shared memory is created with either read or
read/write permissions for both permission types passed in, allowing the
remote permissions to also be set as "don't care".
Part of the checking done by the kernel is to check if the given
address and size are 4KB aligned, as well as checking if the size isn't
zero. It also only allows mapping shared memory as readable or
read/write, but nothing else, and so we shouldn't allow mapping as
anything else either.
Previously, these were sitting outside of the Kernel namespace, which
doesn't really make sense, given they're related to the Thread class
which is within the Kernel namespace.
There were a few places where nested namespace specifiers weren't being
used where they could be within the service code. This amends that to
make the namespacing a tiny bit more compact.
While unlikely, it does avoid constructing a std::string and
unnecessarily calling into the memory code if a game or executable
decides to be really silly about their logging.
This places the font data within cpp files, which mitigates the
possibility of the font data being duplicated within the binary if it's
referred to in more than one translation unit in the future. It also
stores the data within a std::array, which is more flexible when it
comes to operating with the standard library.
Furthermore, it makes the data arrays const. This is what we want, as it
allows the compiler to store the data within the read-only segment. As
it is, having several large sections of mutable data like this just
leaves spots in memory that we can accidentally write to (via accidental
overruns, what have you) and actually have it work. This ensures the
font data remains the same no matter what.
When a destructor isn't defaulted into a cpp file, it can cause the use
of forward declarations to seemingly fail to compile for non-obvious
reasons. It also allows inlining of the construction/destruction logic
all over the place where a constructor or destructor is invoked, which
can lead to code bloat. This isn't so much a worry here, given the
services won't be created and destroyed frequently.
The cause of the above mentioned non-obvious errors can be demonstrated
as follows:
------- Demonstrative example, if you know how the described error happens, skip forwards -------
Assume we have the following in the header, which we'll call "thing.h":
\#include <memory>
// Forward declaration. For example purposes, assume the definition
// of Object is in some header named "object.h"
class Object;
class Thing {
public:
// assume no constructors or destructors are specified here,
// or the constructors/destructors are defined as:
//
// Thing() = default;
// ~Thing() = default;
//
// ... Some interface member functions would be defined here
private:
std::shared_ptr<Object> obj;
};
If this header is included in a cpp file, (which we'll call "main.cpp"),
this will result in a compilation error, because even though no
destructor is specified, the destructor will still need to be generated by
the compiler because std::shared_ptr's destructor is *not* trivial (in
other words, it does something other than nothing), as std::shared_ptr's
destructor needs to do two things:
1. Decrement the shared reference count of the object being pointed to,
and if the reference count decrements to zero,
2. Free the Object instance's memory (aka deallocate the memory it's
pointing to).
And so the compiler generates the code for the destructor doing this inside main.cpp.
Now, keep in mind, the Object forward declaration is not a complete type. All it
does is tell the compiler "a type named Object exists" and allows us to
use the name in certain situations to avoid a header dependency. So the
compiler needs to generate destruction code for Object, but the compiler
doesn't know *how* to destruct it. A forward declaration doesn't tell
the compiler anything about Object's constructor or destructor. So, the
compiler will issue an error in this case because it's undefined
behavior to try and deallocate (or construct) an incomplete type and
std::shared_ptr and std::unique_ptr make sure this isn't the case
internally.
Now, if we had defaulted the destructor in "thing.cpp", where we also
include "object.h", this would never be an issue, as the destructor
would only have its code generated in one place, and it would be in a
place where the full class definition of Object would be visible to the
compiler.
---------------------- End example ----------------------------
Given these service classes are more than certainly going to change in
the future, this defaults the constructors and destructors into the
relevant cpp files to make the construction and destruction of all of
the services consistent and unlikely to run into cases where forward
declarations are indirectly causing compilation errors. It also has the
plus of avoiding the need to rebuild several services if destruction
logic changes, since it would only be necessary to recompile the single
cpp file.
Given we now have the kernel as a class, it doesn't make sense to keep
the current process pointer within the System class, as processes are
related to the kernel.
This also gets rid of a subtle case where memory wouldn't be freed on
core shutdown, as the current_process pointer would never be reset,
causing the pointed to contents to continue to live.
The only reason this include was necessary, was because the constructor
wasn't defaulted in the cpp file and the compiler would inline it
wherever it was used. However, given Controller is forward declared, all
those inlined constructors would see an incomplete type, causing a
compilation failure. So, we just place the constructor in the cpp file,
where it can see the complete type definition, allowing us to remove
this include.
