[Lldb-commits] [lldb] db9322b - [LLDB][NFC] Reliability fixes for ObjectFileMachO.cpp

[Slava Gurevich via lldb-commits]

Author: Slava Gurevich
Date: 2022-08-10T15:16:37-07:00
New Revision: db9322b2066c55254e7691efeab863f43bfcc084

URL: https://github.com/llvm/llvm-project/commit/db9322b2066c55254e7691efeab863f43bfcc084
DIFF: https://github.com/llvm/llvm-project/commit/db9322b2066c55254e7691efeab863f43bfcc084.diff

LOG: [LLDB][NFC] Reliability fixes for ObjectFileMachO.cpp

Static code inspection guided fixes for the following issues:
- dead code
- buffer not null-terminated
- null-dereference
- out-of-bounds access

Differential Revision: https://reviews.llvm.org/D131554

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 73597ae3fb183..e4832dad1e476 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -533,8 +533,13 @@ class RegisterContextDarwin_arm_Mach : public RegisterContextDarwin_arm {
       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) {
-          gpr.r[i] = data.GetU32(&offset);
+
+        // Prevent static analysis warnings by explicitly contstraining 'count'
+        // to acceptable range. Handle possible underflow of count-1
+        if (count > 0 && count <= sizeof(gpr.r) / sizeof(gpr.r[0])) {
+          for (uint32_t i = 0; i < (count - 1); ++i) {
+            gpr.r[i] = data.GetU32(&offset);
+          }
         }
         // Save cpsr explicitly.
         gpr.cpsr = data.GetU32(&offset);
@@ -544,7 +549,7 @@ class RegisterContextDarwin_arm_Mach : public RegisterContextDarwin_arm {
         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 +4121,9 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
             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;
@@ -6335,6 +6341,11 @@ static offset_t CreateAllImageInfosPayload(
           continue;
         ConstString name = section->GetName();
         segment_vmaddr seg_vmaddr;
+        // This is the uncommon case where strncpy is exactly
+        // the right one, doesn't need to be nul terminated.
+        // 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.
         strncpy(seg_vmaddr.segname, name.AsCString(),
                 sizeof(seg_vmaddr.segname));
         seg_vmaddr.vmaddr = vmaddr;
@@ -6726,8 +6737,10 @@ bool ObjectFileMachO::SaveCore(const lldb::ProcessSP &process_sp,
           buffer.PutHex32(sizeof(llvm::MachO::note_command));
           char namebuf[16];
           memset(namebuf, 0, sizeof(namebuf));
-          // this is the uncommon case where strncpy is exactly
+          // This is the uncommon case where strncpy is exactly
           // the right one, doesn't need to be nul terminated.
+          // LC_NOTE name field is char[16] and is not guaranteed to be
+          // nul-terminated.
           strncpy(namebuf, lcnote->name.c_str(), sizeof(namebuf));
           buffer.PutRawBytes(namebuf, sizeof(namebuf));
           buffer.PutHex64(lcnote->payload_file_offset);
@@ -6885,8 +6898,10 @@ ObjectFileMachO::GetCorefileAllImageInfos() {
         }
         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++) {