Skip to content

[lldb] Skip null bytes in embedded type summaries #8132

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

Conversation

kastiglione
Copy link

@kastiglione kastiglione commented Feb 6, 2024

Handle null padding that may exists between embedded type summary records. This can happen for example on x86-64 where the default alignment of char[] is 16 (p2align = 4).

@kastiglione
Copy link
Author

@swift-ci test macOS

@kastiglione
Copy link
Author

@swift-ci test linux

@kastiglione
Copy link
Author

@swift-ci test windows

// Skip null bytes. Can happen with alignment padding.
while (true) {
auto next_offset = offset;
if (extractor.GetU8(&next_offset) != 0) {

Choose a reason for hiding this comment

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

Why is next_offset needed? Does GetU8 modify the parameter in the failure case?

Copy link
Author

Choose a reason for hiding this comment

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

yes, the passed in offset is an in-out parameter. As far as GetU8 is concerned, reading a zero byte is not a failure case. If there was a PeekU8 method that would be better suited.

Copy link
Author

Choose a reason for hiding this comment

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

open to suggestions

Choose a reason for hiding this comment

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

No this is fine. I misread the condition, as checking for an error, but it's checking for the NUL byte.

// Skip null bytes. Can happen with alignment padding.
while (true) {
auto next_offset = offset;
if (extractor.GetU8(&next_offset) != 0) {

Choose a reason for hiding this comment

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

No this is fine. I misread the condition, as checking for an error, but it's checking for the NUL byte.

@kastiglione kastiglione merged commit 87ace14 into stable/20230725 Feb 8, 2024
@kastiglione kastiglione deleted the dl/lldb-Skip-null-bytes-in-embedded-type-summaries branch February 8, 2024 17:33
kastiglione added a commit that referenced this pull request Feb 16, 2024
Handle null padding that may exists between embedded type summary records. This can 
happen for example on x86-64 where the default alignment of `char[]` is 16 (p2align = 
4).

(cherry-picked from commit 87ace14)
kastiglione added a commit that referenced this pull request Feb 16, 2024
Handle null padding that may exists between embedded type summary records. This can 
happen for example on x86-64 where the default alignment of `char[]` is 16 (p2align = 
4).

(cherry-picked from commit 87ace14)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants