Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LibraryPathDiagnostics prints UUIDs with bytes in the reverse order #3391

Merged
merged 1 commit into from Aug 18, 2022

Conversation

davidquesada
Copy link
Contributor

@davidquesada davidquesada commented Aug 17, 2022

2fe54ca

LibraryPathDiagnostics prints UUIDs with bytes in the reverse order
https://bugs.webkit.org/show_bug.cgi?id=244011
rdar://98737098

Reviewed by Saam Barati.

In https://bugs.webkit.org/show_bug.cgi?id=243458, I added code in LibraryPathDiagnostics that
attempts to convert `uuid_t` returned by DYLD API to `WTF::UUID`, but I didn't realize that these
two types don't agree on endianness -- the former appears to be big-endian while the latter is
little-endian. The natural consequence of this is that the UUIDs printed in the logs are shown
with their bytes in the reverse order of what they should be.

Fix this by removing the conversion between the two UUID types altogether, and simply use the
platform API `uuid_unparse()` to produce the string representations of `uuid_t`s. Considering
how infrequently this type is used in the WebKit codebase (this is the second usage of this
type, other than ProcessLauncherCocoa), I chose to implement this as a local helper function
rather than building this conversion into WTF's UUID type directly. Doing so would require
either building in the assumption that `uuid_t` is stored big-endian on all platforms, or we'd
need to use an inefficient implementation that performs this conversion by round-tripping the
UUID through a string representation. This conversion isn't actually needed at the moment, so
I'd rather not do either of those when the tool already exists to do what *actually* needs to
be done -- convert the `uuid_t` to a string.

* Source/WTF/wtf/darwin/LibraryPathDiagnostics.mm:
(WTF::uuidToString):
(WTF::LibraryPathDiagnosticsLogger::logDYLDSharedCacheInfo):
(WTF::LibraryPathDiagnosticsLogger::logDynamicLibraryInfo):

Canonical link: https://commits.webkit.org/253545@main

Copy link
Contributor

@saambarati saambarati left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me

@davidquesada davidquesada added the merge-queue Applied to send a pull request to merge-queue label Aug 18, 2022
https://bugs.webkit.org/show_bug.cgi?id=244011
rdar://98737098

Reviewed by Saam Barati.

In https://bugs.webkit.org/show_bug.cgi?id=243458, I added code in LibraryPathDiagnostics that
attempts to convert `uuid_t` returned by DYLD API to `WTF::UUID`, but I didn't realize that these
two types don't agree on endianness -- the former appears to be big-endian while the latter is
little-endian. The natural consequence of this is that the UUIDs printed in the logs are shown
with their bytes in the reverse order of what they should be.

Fix this by removing the conversion between the two UUID types altogether, and simply use the
platform API `uuid_unparse()` to produce the string representations of `uuid_t`s. Considering
how infrequently this type is used in the WebKit codebase (this is the second usage of this
type, other than ProcessLauncherCocoa), I chose to implement this as a local helper function
rather than building this conversion into WTF's UUID type directly. Doing so would require
either building in the assumption that `uuid_t` is stored big-endian on all platforms, or we'd
need to use an inefficient implementation that performs this conversion by round-tripping the
UUID through a string representation. This conversion isn't actually needed at the moment, so
I'd rather not do either of those when the tool already exists to do what *actually* needs to
be done -- convert the `uuid_t` to a string.

* Source/WTF/wtf/darwin/LibraryPathDiagnostics.mm:
(WTF::uuidToString):
(WTF::LibraryPathDiagnosticsLogger::logDYLDSharedCacheInfo):
(WTF::LibraryPathDiagnosticsLogger::logDynamicLibraryInfo):

Canonical link: https://commits.webkit.org/253545@main
@webkit-commit-queue
Copy link
Collaborator

Committed 253545@main (2fe54ca): https://commits.webkit.org/253545@main

Reviewed commits have been landed. Closing PR #3391 and removing active labels.

@webkit-early-warning-system webkit-early-warning-system merged commit 2fe54ca into WebKit:main Aug 18, 2022
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants