Skip to content

Commit

Permalink
Use the dynamic linker's data directly and copy it C# side immediately (
Browse files Browse the repository at this point in the history 
#848)
  • Loading branch information
@kstenerud
kstenerud authored Nov 7, 2024
1 parent 1c8974f commit 55b2db6
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 161 deletions.
143 changes: 26 additions & 117 deletions src/BugsnagUnity.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,146 +6,50 @@

extern "C" {

static uint8_t *byteDupOrNull(const uint8_t * const oldBytes, int byteCount) {
auto newBytes = new uint8_t[byteCount];
if (oldBytes != nullptr) {
memcpy(newBytes, oldBytes, byteCount);
}
return newBytes;
}

static char *strDupOrNull(const char* const oldStr) {
if (oldStr == nullptr) {
return nullptr;
}
auto byteCount = strnlen(oldStr, 1000);
auto newStr = new char[byteCount + 1];
memcpy(newStr, oldStr, byteCount);
newStr[byteCount] = 0;
return newStr;
}

struct bugsnag_user {
const char *user_id;
const char *user_name;
const char *user_email;
};

class LoadedImage {

class NativeLoadedImage {
public:
// We need to expose raw pointers and blittable types for native interop with C#
// https://learn.microsoft.com/en-us/dotnet/standard/native-interop/best-practices#blittable-types
// These members must be in the EXACT SAME ORDER and of the EXACT SAME SIZE as the C# side!
uint64_t LoadAddress{0};
uint64_t ImageSize{0};
// These are pointers to data allocated by the dynamic loader, and could be deallocated at any time.
const char *FileName{nullptr};
uint8_t *UuidBytes{nullptr};

public:
// "invalid" image
LoadedImage() {}

LoadedImage(const struct mach_header * const header);

LoadedImage(const char * const filename,
const uint8_t * const uuidBytes,
const uint64_t loadAddress,
const uint64_t imageSize)
: FileName(strDupOrNull(filename))
, UuidBytes(byteDupOrNull(uuidBytes, 16))
, LoadAddress(loadAddress)
, ImageSize(imageSize)
{}

LoadedImage(LoadedImage&& other)
: FileName(other.FileName)
, UuidBytes(other.UuidBytes)
, LoadAddress(other.LoadAddress)
, ImageSize(other.ImageSize)
{
other.invalidateAndAbandonOwnership();
}

LoadedImage(const LoadedImage& other)
: LoadedImage(other.FileName, other.UuidBytes, other.LoadAddress, other.ImageSize)
{}

~LoadedImage() {
releaseResources();
}

LoadedImage *clone() {
return new LoadedImage(FileName, UuidBytes, LoadAddress, ImageSize);
}

LoadedImage& operator=(LoadedImage&& other) {
if(this != &other) {
delete [] FileName;
delete [] UuidBytes;
// Assume ownership of these
FileName = other.FileName;
UuidBytes = other.UuidBytes;
LoadAddress = other.LoadAddress;
ImageSize = other.ImageSize;
other.invalidateAndAbandonOwnership();
}
return *this;
}
NativeLoadedImage() {}

LoadedImage& operator=(const LoadedImage& other)
{
if(this != &other) {
delete [] FileName;
delete [] UuidBytes;
// Make copies of these
FileName = strDupOrNull(other.FileName);
UuidBytes = byteDupOrNull(other.UuidBytes, 16);
LoadAddress = other.LoadAddress;
ImageSize = other.ImageSize;
}
return *this;
}
NativeLoadedImage(const struct mach_header * const header);

bool isValid() const {
return ImageSize > 0;
}

private:
void invalidateAndAbandonOwnership() {
FileName = nullptr;
UuidBytes = nullptr;
LoadAddress = 0;
ImageSize = 0;
}

void releaseResources() {
if (FileName != nullptr) {
delete [] FileName;
FileName = nullptr;
}
if (UuidBytes != nullptr) {
delete [] UuidBytes;
UuidBytes = nullptr;
}
}
};

bool operator<(const LoadedImage& lhs, const LoadedImage& rhs)
bool operator<(const NativeLoadedImage& lhs, const NativeLoadedImage& rhs)
{
return lhs.LoadAddress < rhs.LoadAddress;
}
bool operator>(const LoadedImage& lhs, const LoadedImage& rhs)
bool operator>(const NativeLoadedImage& lhs, const NativeLoadedImage& rhs)
{
return lhs.LoadAddress > rhs.LoadAddress;
}
bool operator==(const LoadedImage& lhs, const LoadedImage& rhs)
bool operator==(const NativeLoadedImage& lhs, const NativeLoadedImage& rhs)
{
return lhs.LoadAddress == rhs.LoadAddress;
}

LoadedImage::LoadedImage(const struct mach_header * const header) {
NativeLoadedImage::NativeLoadedImage(const struct mach_header * const header) {
if (header == NULL) {
LoadedImage();
NativeLoadedImage();
return;
}

Expand All @@ -161,21 +65,21 @@ void releaseResources() {
break;
default:
// Header is corrupt
LoadedImage();
NativeLoadedImage();
return;
}

Dl_info dlInfo = {0};
if (dladdr(header, &dlInfo) == 0) {
LoadedImage();
NativeLoadedImage();
return;
}

const char *fileName = dlInfo.dli_fname;
const uint64_t loadAddress = (uint64_t)dlInfo.dli_fbase;

if (fileName == nullptr) {
LoadedImage();
NativeLoadedImage();
return;
}

Expand Down Expand Up @@ -217,11 +121,11 @@ void releaseResources() {
}

// All currently loaded images. This MUST be kept ordered: LoadAddress low to high.
static std::vector<LoadedImage> allImages;
static std::vector<NativeLoadedImage> allImages;
static std::mutex allImagesMutex;

static void add_image(const struct mach_header *header, intptr_t slide) {
LoadedImage image(header);
NativeLoadedImage image(header);
if (!image.isValid()) {
return;
}
Expand All @@ -231,7 +135,7 @@ static void add_image(const struct mach_header *header, intptr_t slide) {
}

static void remove_image(const struct mach_header *header, intptr_t slide) {
LoadedImage image(header);
NativeLoadedImage image(header);
if (!image.isValid()) {
return;
}
Expand Down Expand Up @@ -259,18 +163,23 @@ uint64_t bugsnag_getLoadedImageCount() {
return allImages.size();
}

uint64_t bugsnag_getLoadedImages(LoadedImage *images, uint64_t capacity) {
std::lock_guard<std::mutex> guard(allImagesMutex);
uint64_t bugsnag_getLoadedImages(NativeLoadedImage *images, uint64_t capacity) {
// Lock and hold the lock until bugsnag_unlockLoadedImages() is called.
// We do this to allow the C# side time to copy over FileName and UUID.
allImagesMutex.lock();
uint64_t count = allImages.size();
if (count > capacity) {
count = capacity;
}
for(uint64_t i = 0; i < count; i++) {
images[i] = allImages[i];
}
memcpy(images, allImages.data(), sizeof(*images)*count);

return count;
}

void bugsnag_unlockLoadedImages() {
allImagesMutex.unlock();
}

// ==========================================================================================================
// ==========================================================================================================
// ==========================================================================================================
Expand Down
12 changes: 12 additions & 0 deletions src/BugsnagUnity/Native/Cocoa/NativeCode.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,25 @@ partial class NativeCode
/// <param name="capacity">The number of entries in images.</param>
/// <returns>The number of entries that were filled.</returns>
/// <remarks>
/// You MUST call bugsnag_unlockLoadedImages() after you are done with the NativeLoadedImage structs.
///
/// Note: Array types MUST be declared as [In, Out] or else you'll get all sorts of weird issues,
/// like arrays being truncated to zero length, or a single entry being duplicated across the
/// entire array. The compiler won't complain, either.
/// </remarks>
[DllImport(Import)]
internal static extern UInt64 bugsnag_getLoadedImages([In, Out] NativeLoadedImage[] images, UInt64 capacity);

/// <summary>
/// Unlock the mutex protecting the loaded images list.
/// </summary>
/// <remarks>
/// This MUST be called after collecting the data from tha NativeLoadedImage structs returned by
/// bugsnag_getLoadedImages().
/// </remarks>
[DllImport(Import)]
internal static extern void bugsnag_unlockLoadedImages();

[DllImport(Import)]
internal static extern void bugsnag_startBugsnagWithConfiguration(IntPtr configuration, string notifierVersion);

Expand Down
56 changes: 12 additions & 44 deletions src/BugsnagUnity/Native/Cocoa/NativeImage.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,56 +4,22 @@

namespace BugsnagUnity
{
/// <summary>
/// Wraps a NativeLoadedImage to provide automatic and lazy marshaling from the native side.
/// </summary>
/// <remarks>
/// We generally don't need the name and UUID except in specific cases, so we unmarshal
/// them lazily.
/// </remarks>
class LoadedImage
{
public UInt64 LoadAddress
{
get => Image.LoadAddress;
}
public UInt64 Size
{
get => Image.Size;
}
public string FileName
{
get
{
if (CachedFileName == null)
{
CachedFileName = Marshal.PtrToStringAnsi(Image.FileName);
}
return CachedFileName;
}
}
public string Uuid
{
get
{
if (CachedUuid == null)
{
var uuid = new byte[16];
Marshal.Copy(Image.UuidBytes, uuid, 0, 16);
CachedUuid = new Guid(uuid).ToString();
}
return CachedUuid;
}
}

public LoadedImage(NativeLoadedImage image)
{
Image = image;
LoadAddress = image.LoadAddress;
Size = image.Size;
FileName = Marshal.PtrToStringAnsi(image.FileName);
var uuid = new byte[16];
Marshal.Copy(image.UuidBytes, uuid, 0, 16);
Uuid = new Guid(uuid).ToString();
}

private NativeLoadedImage Image;
private string CachedFileName;
private string CachedUuid;
public UInt64 LoadAddress;
public UInt64 Size;
public string FileName;
public string Uuid;
}

class LoadedImages
Expand All @@ -75,6 +41,8 @@ public void Refresh()
images[i] = new LoadedImage(nativeImages[i]);
}
Images = images;
// bugsnag_getLoadedImages() locks a mutex, so we must call bugsnag_unlockLoadedImages()
NativeCode.bugsnag_unlockLoadedImages();
}

/// <summary>
Expand Down

0 comments on commit 55b2db6

Please sign in to comment.