[Lldb-commits] [lldb] [lldb] [Mach-O] don't strip the end of the "kern ver str" LC_NOTE (PR #77538)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Jan 9 15:05:14 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Jason Molenda (jasonmolenda)
<details>
<summary>Changes</summary>
The "kern ver str" LC_NOTE gives lldb a kernel version string -- with a UUID and/or a load address (stext) to load it at. The LC_NOTE specifies a size of the identifier string in bytes. In ObjectFileMachO::GetIdentifierString, I copy that number of bytes into a std::string, and in case there were additional nul characters at the end of the sting for padding reasons, I tried to shrink the std::string to not include these extra nul's.
However, I did this resizing without handling the case of an empty identifier string. I don't know why any corefile creator would do that, but of course at least one does. This patch removes the resizing altogether; I was solving something that hasn't ever shown to be a problem. I also added a test case for this, to check that lldb doesn't crash when given one of these corefiles.
rdar://120390199
---
Full diff: https://github.com/llvm/llvm-project/pull/77538.diff
3 Files Affected:
- (modified) lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp (-4)
- (modified) lldb/test/API/macosx/lc-note/firmware-corefile/TestFirmwareCorefiles.py (+23-1)
- (modified) lldb/test/API/macosx/lc-note/firmware-corefile/create-empty-corefile.cpp (+17-8)
``````````diff
diff --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 182a9f2afaeb2e..d7a2846200fcaf 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -5467,8 +5467,6 @@ std::string ObjectFileMachO::GetIdentifierString() {
uint32_t strsize = payload_size - sizeof(uint32_t);
std::string result(strsize, '\0');
m_data.CopyData(payload_offset, strsize, result.data());
- while (result.back() == '\0')
- result.resize(result.size() - 1);
LLDB_LOGF(log, "LC_NOTE 'kern ver str' found with text '%s'",
result.c_str());
return result;
@@ -5488,8 +5486,6 @@ std::string ObjectFileMachO::GetIdentifierString() {
std::string result(ident_command.cmdsize, '\0');
if (m_data.CopyData(offset, ident_command.cmdsize, result.data()) ==
ident_command.cmdsize) {
- while (result.back() == '\0')
- result.resize(result.size() - 1);
LLDB_LOGF(log, "LC_IDENT found with text '%s'", result.c_str());
return result;
}
diff --git a/lldb/test/API/macosx/lc-note/firmware-corefile/TestFirmwareCorefiles.py b/lldb/test/API/macosx/lc-note/firmware-corefile/TestFirmwareCorefiles.py
index 9fd12c3ba49c29..b9d2055e83a56c 100644
--- a/lldb/test/API/macosx/lc-note/firmware-corefile/TestFirmwareCorefiles.py
+++ b/lldb/test/API/macosx/lc-note/firmware-corefile/TestFirmwareCorefiles.py
@@ -24,6 +24,9 @@ def test_lc_note_version_string(self):
aout_exe_basename = "a.out"
aout_exe = self.getBuildArtifact(aout_exe_basename)
verstr_corefile = self.getBuildArtifact("verstr.core")
+ verstr_corefile_invalid_ident = self.getBuildArtifact(
+ "verstr-invalid-ident.core"
+ )
verstr_corefile_addr = self.getBuildArtifact("verstr-addr.core")
create_corefile = self.getBuildArtifact("create-empty-corefile")
slide = 0x70000000000
@@ -36,6 +39,14 @@ def test_lc_note_version_string(self):
+ " 0xffffffffffffffff 0xffffffffffffffff",
shell=True,
)
+ call(
+ create_corefile
+ + " version-string "
+ + verstr_corefile_invalid_ident
+ + ' "" '
+ + "0xffffffffffffffff 0xffffffffffffffff",
+ shell=True,
+ )
call(
create_corefile
+ " version-string "
@@ -71,7 +82,18 @@ def test_lc_note_version_string(self):
self.assertEqual(fspec.GetFilename(), aout_exe_basename)
self.dbg.DeleteTarget(target)
- # Second, try the "kern ver str" corefile where it loads at an address
+ # Second, try the "kern ver str" corefile which has an invalid ident,
+ # make sure we don't crash.
+ target = self.dbg.CreateTarget("")
+ err = lldb.SBError()
+ if self.TraceOn():
+ self.runCmd(
+ "script print('loading corefile %s')" % verstr_corefile_invalid_ident
+ )
+ process = target.LoadCore(verstr_corefile_invalid_ident)
+ self.assertEqual(process.IsValid(), True)
+
+ # Third, try the "kern ver str" corefile where it loads at an address
target = self.dbg.CreateTarget("")
err = lldb.SBError()
if self.TraceOn():
diff --git a/lldb/test/API/macosx/lc-note/firmware-corefile/create-empty-corefile.cpp b/lldb/test/API/macosx/lc-note/firmware-corefile/create-empty-corefile.cpp
index 8bd6aaabecd636..d7c2d422412e42 100644
--- a/lldb/test/API/macosx/lc-note/firmware-corefile/create-empty-corefile.cpp
+++ b/lldb/test/API/macosx/lc-note/firmware-corefile/create-empty-corefile.cpp
@@ -86,14 +86,16 @@ std::vector<uint8_t> lc_thread_load_command(cpu_type_t cputype) {
void add_lc_note_kern_ver_str_load_command(
std::vector<std::vector<uint8_t>> &loadcmds, std::vector<uint8_t> &payload,
int payload_file_offset, std::string uuid, uint64_t address) {
- std::string ident = "EFI UUID=";
- ident += uuid;
-
- if (address != 0xffffffffffffffff) {
- ident += "; stext=";
- char buf[24];
- sprintf(buf, "0x%" PRIx64, address);
- ident += buf;
+ std::string ident;
+ if (!uuid.empty()) {
+ ident = "EFI UUID=";
+ ident += uuid;
+ if (address != 0xffffffffffffffff) {
+ ident += "; stext=";
+ char buf[24];
+ sprintf(buf, "0x%" PRIx64, address);
+ ident += buf;
+ }
}
std::vector<uint8_t> loadcmd_data;
@@ -187,6 +189,9 @@ void add_lc_segment(std::vector<std::vector<uint8_t>> &loadcmds,
std::string get_uuid_from_binary(const char *fn, cpu_type_t &cputype,
cpu_subtype_t &cpusubtype) {
+ if (strlen(fn) == 0)
+ return {};
+
FILE *f = fopen(fn, "r");
if (f == nullptr) {
fprintf(stderr, "Unable to open binary '%s' to get uuid\n", fn);
@@ -295,6 +300,10 @@ int main(int argc, char **argv) {
fprintf(stderr, "an LC_NOTE 'main bin spec' load command without an "
"address specified, depending on\n");
fprintf(stderr, "whether the 1st arg is version-string or main-bin-spec\n");
+ fprintf(stderr, "\nan LC_NOTE 'kern ver str' with no binary provided "
+ "(empty string filename) to get a UUID\n");
+ fprintf(stderr, "means an empty 'kern ver str' will be written, an invalid "
+ "LC_NOTE that lldb should handle.\n");
exit(1);
}
if (strcmp(argv[1], "version-string") != 0 &&
``````````
</details>
https://github.com/llvm/llvm-project/pull/77538
More information about the lldb-commits
mailing list