jasonmolenda accepted this revision.
jasonmolenda added a comment.
This revision is now accepted and ready to land.
Looks good to me with a few suggestions inlined.
================
Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:537
+
+ // Prevent static analysis warnings by explicitly contsraining 'count'
+ // to acceptable range. Handle possible underflow of count-1
----------------
'constraining'
================
Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:540
+ for (uint32_t i = 0;
+ count > 0 && count <= sizeof(gpr.r) && i < count - 1; ++i) {
gpr.r[i] = data.GetU32(&offset);
----------------
The `count` field for a Darwin register context is the number of 4-byte entries
in the object - it's a trick the kernel API often use so they can add fields
later and the kernel knows what version of the object the userland process is
requesting when it asks for "flavor, size" in a `get_thread_state` call. This
Aarch32 register context is `struct GPR {uint32_t r[16]; uint32_t cpsr};` or
count 17, but `sizeof(gpr.r)` is going to be 64. We only want to loop for 16
entries.
================
Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:6345
sizeof(seg_vmaddr.segname));
+ seg_vmaddr.segname[sizeof(seg_vmaddr.segname) - 1] = '\0';
seg_vmaddr.vmaddr = vmaddr;
----------------
fixathon wrote:
> strncpy may not have null-termination. Please comment if you believe this
> should not be corrected. I'd like to at least add the comments with
> sufficient explanation if that is the case.
Yes, the segment name in a Mach-O LC_SEGMENT/LC_SEGMENT_64 is char[16] and is
not guaranteed to be nul terminated if all 16 characters are used. This code
shouldn't be changed - adding a comment to that effect would be fine though.
================
Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:6737-6738
// the right one, doesn't need to be nul terminated.
strncpy(namebuf, lcnote->name.c_str(), sizeof(namebuf));
+ namebuf[sizeof(namebuf) - 1] = '\0';
buffer.PutRawBytes(namebuf, sizeof(namebuf));
----------------
fixathon wrote:
> strncpy may not have null-termination. Please comment if you believe this
> should not be corrected. I'd like to at least add the comments with
> sufficient explanation if that is the case.
Same thing here - LC_NOTE name field is char[16] and is not guaranteed to be
nul terminated.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131554/new/
https://reviews.llvm.org/D131554
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
