-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[LLDB][Minidump] Fix bug in generating 64b memory minidumps #141995
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
… 64b memory ranges on Minidumps
@llvm/pr-subscribers-lldb Author: Jacob Lalonde (Jlalond) ChangesIn #129307, we introduced read write in chunks, and during the final revision of the PR I changed the behavior for 64b memory regions and did not test an actual 64b memory range. This caused LLDB to crash whenever we generated a 64b memory region. 64b regions has been a problem in testing for some time as it's a waste of test resources to generation a 5gb+ Minidump. I will work with @clayborg and @labath to come up with a way to specify creating a 64b list instead of a 32b list (likely via the yamilizer). Full diff: https://github.com/llvm/llvm-project/pull/141995.diff 1 Files Affected:
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index 2818d31eb2301..806f256d9da48 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -1170,7 +1170,6 @@ MinidumpFileBuilder::AddMemoryList_64(std::vector<CoreFileMemoryRange> &ranges,
"(%" PRIx64 "bytes) "
"[%" PRIx64 ", %" PRIx64 ")",
region_index, ranges.size(), size, addr, addr + size);
- ++region_index;
progress.Increment(1, "Adding Memory Range " + core_range.Dump());
uint64_t bytes_read = 0;
@@ -1186,6 +1185,8 @@ MinidumpFileBuilder::AddMemoryList_64(std::vector<CoreFileMemoryRange> &ranges,
cleanup_required = true;
descriptors[region_index].DataSize = bytes_read;
}
+
+ ++region_index;
}
// Early return if there is no cleanup needed.
|
Tested against a process with 64b memory |
In #129307, we introduced read write in chunks, and during the final revision of the PR I changed the behavior for 64b memory regions and did not test an actual 64b memory range. This caused LLDB to crash whenever we generated a 64b memory region. 64b regions has been a problem in testing for some time as it's a waste of test resources to generation a 5gb+ Minidump. I will work with @clayborg and @labath to come up with a way to specify creating a 64b list instead of a 32b list (likely via the yamilizer).
) In llvm#129307, we introduced read write in chunks, and during the final revision of the PR I changed the behavior for 64b memory regions and did not test an actual 64b memory range. This caused LLDB to crash whenever we generated a 64b memory region. 64b regions has been a problem in testing for some time as it's a waste of test resources to generation a 5gb+ Minidump. I will work with @clayborg and @labath to come up with a way to specify creating a 64b list instead of a 32b list (likely via the yamilizer).
) In llvm#129307, we introduced read write in chunks, and during the final revision of the PR I changed the behavior for 64b memory regions and did not test an actual 64b memory range. This caused LLDB to crash whenever we generated a 64b memory region. 64b regions has been a problem in testing for some time as it's a waste of test resources to generation a 5gb+ Minidump. I will work with @clayborg and @labath to come up with a way to specify creating a 64b list instead of a 32b list (likely via the yamilizer).
In #129307, we introduced read write in chunks, and during the final revision of the PR I changed the behavior for 64b memory regions and did not test an actual 64b memory range.
This caused LLDB to crash whenever we generated a 64b memory region.
64b regions has been a problem in testing for some time as it's a waste of test resources to generation a 5gb+ Minidump. I will work with @clayborg and @labath to come up with a way to specify creating a 64b list instead of a 32b list (likely via the yamilizer).