[Lldb-commits] [PATCH] D131554: [LLDB][NFC] Reliability fixes for ObjectFileMachO.cpp

[Slava Gurevich via Phabricator via lldb-commits]

fixathon created this revision.
fixathon added reviewers: clayborg, JDevlieghere, DavidSpickett, jasonmolenda.
Herald added a project: All.
fixathon requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Static code inspection guided fixes for the following issues:

- dead code
- buffer not null-terminated
- null-dereference
- out-of-bounds access


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131554

Files:
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp


Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===================================================================
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -533,7 +533,11 @@
       case GPRRegSet:
         // On ARM, the CPSR register is also included in the count but it is
         // not included in gpr.r so loop until (count-1).
-        for (uint32_t i = 0; i < (count - 1); ++i) {
+
+        // Prevent static analysis warnings by explicitly contsraining 'count'
+        // to acceptable range. Handle possible underflow of count-1
+        for (uint32_t i = 0;
+             count > 0 && count <= sizeof(gpr.r) && i < count - 1; ++i) {
           gpr.r[i] = data.GetU32(&offset);
         }
         // Save cpsr explicitly.
@@ -544,7 +548,7 @@
         break;
 
       case FPURegSet: {
-        uint8_t *fpu_reg_buf = (uint8_t *)&fpu.floats.s[0];
+        uint8_t *fpu_reg_buf = (uint8_t *)&fpu.floats;
         const int fpu_reg_buf_size = sizeof(fpu.floats);
         if (data.ExtractBytes(offset, fpu_reg_buf_size, eByteOrderLittle,
                               fpu_reg_buf) == fpu_reg_buf_size) {
@@ -4116,8 +4120,9 @@
             sym[sym_idx].SetReExportedSymbolName(reexport_name);
             set_value = false;
             reexport_shlib_needs_fixup[sym_idx] = reexport_name;
-            indirect_symbol_names.insert(
-                ConstString(symbol_name + ((symbol_name[0] == '_') ? 1 : 0)));
+            indirect_symbol_names.insert(ConstString(
+                symbol_name +
+                ((symbol_name && (symbol_name[0] == '_')) ? 1 : 0)));
           } else
             type = eSymbolTypeUndefined;
         } break;
@@ -6337,6 +6342,7 @@
         segment_vmaddr seg_vmaddr;
         strncpy(seg_vmaddr.segname, name.AsCString(),
                 sizeof(seg_vmaddr.segname));
+        seg_vmaddr.segname[sizeof(seg_vmaddr.segname) - 1] = '\0';
         seg_vmaddr.vmaddr = vmaddr;
         seg_vmaddr.unused = 0;
         segment_vmaddrs.push_back(seg_vmaddr);
@@ -6729,6 +6735,7 @@
           // this is the uncommon case where strncpy is exactly
           // 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));
           buffer.PutHex64(lcnote->payload_file_offset);
           buffer.PutHex64(lcnote->payload.GetSize());
@@ -6885,8 +6892,10 @@
         }
         uint32_t imgcount = m_data.GetU32(&offset);
         uint64_t entries_fileoff = m_data.GetU64(&offset);
-        offset += 4; // uint32_t entries_size;
-        offset += 4; // uint32_t unused;
+        /* leaving the following dead code as comments for spec documentation
+            offset += 4; // uint32_t entries_size;
+            offset += 4; // uint32_t unused;
+        */
 
         offset = entries_fileoff;
         for (uint32_t i = 0; i < imgcount; i++) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D131554.451402.patch
Type: text/x-patch
Size: 3056 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20220810/a81dc665/attachment.bin>