clayborg added inline comments.
================
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);
----------------
================
Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:541
+ count > 0 && count <= sizeof(gpr.r) && i < count - 1; ++i) {
gpr.r[i] = data.GetU32(&offset);
}
----------------
Just a quick FYI: if the "offset" value ends up being out of bounds for the
data, it will stop parsing anything and set the offset value to UINT64_MAX, so
this is safe already.
================
Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:4116
const char *reexport_name_cstr = strtab_data.PeekCStr(nlist.n_value);
if (reexport_name_cstr && reexport_name_cstr[0]) {
type = eSymbolTypeReExported;
----------------
================
Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:4125
+ symbol_name +
+ ((symbol_name && (symbol_name[0] == '_')) ? 1 : 0)));
} else
----------------
fixathon wrote:
> potential null dereference
Revert, see above code suggestion.
================
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.
Need to revert this. If the segment name is actually 16 characters long, you
will change the segment name which we can't do and there is no NULL termination
in this case.
================
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));
----------------
jasonmolenda wrote:
> 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.
Revert this. If the name is actually 16 characters long it isn't supposed to be
null terminated.
================
Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:6900
offset = entries_fileoff;
for (uint32_t i = 0; i < imgcount; i++) {
----------------
fixathon wrote:
> offset is re-assigned here, so there's no point in the previous 2 lines.
> However it is likely those follow the decoding format specification, so left
> it in as comments for clarity.
Maybe change to something like:
```
// entries_size is not used, nor is the unused entry.
// uint32_t entries_size = m_data.GetU32(&offset);
// uint32_t unused = m_data.GetU32(&offset);
```
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
