Set error status in ObjectFile::LoadInMemory if it is not set

[tatyana-krasnukha]

Summary

When breakpoints intersect a section, process->WriteMemory returns 0 without setting the error, LoadInMemory breaks execution and returns this error, but client code treat it as success.

Just added setting of error status for this case.

Applied clang-format.

[clayborg]

Before working on a file for a diff you will submit, clang format it first, and then check it in. No need for review on clang format only changes.

So please clang format first and check in those change and update this with only the changes you intend to make to the functionality of the code.

[tatyana-krasnukha]

Removed clang-format changes.

[abidh]

source/Symbol/ObjectFile.cpp

695

If WriteMemory is not setting the error in some case, it seem more logical to properly handle it there instead of putting this specific error message here.

[tatyana-krasnukha]

source/Symbol/ObjectFile.cpp

695

Process::WriteMemory doesn't treat this case as error. This depends on context of usage. I wanted to do as you propose, but was afraid for breaking all other use cases...

[tatyana-krasnukha]

ping

Greg, Abid, if you disagree with the changes, let me know and I'll close the revision.

[clayborg]

I am not sure we can say for sure that a breakpoint intersected with the write here? I would rather just have an error saying something like "only %u of %u bytes in section %s were written". Extra credit for checking if there are overlapping breakpoints and appending "(try disabling all breakpoints and loading again)" to the error message.

[tatyana-krasnukha]

Yes, I see... There is some inconsistency between status and returned value. That is why I wondered about this https://reviews.llvm.org/D39967#984065

[clayborg]

And yes, if the write memory can only write fewer byte than requested, it won't be an error if at least some bytes were written

source/Symbol/ObjectFile.cpp

         // and object container plug-ins can use these bytes to see if they
         // can parse this file.
         if (file_size > 0) {
-          data_sp =
-              DataBufferLLVM::CreateSliceFromPath(file->GetPath(), 512, file_offset);
+          data_sp = DataBufferLLVM::CreateSliceFromPath(file->GetPath(), 512,
+                                                        file_offset);
           data_offset = 0;
         }
       }

             }
             // We failed to find any cached object files in the container
             // plug-ins, so lets read the first 512 bytes and try again below...
-            data_sp = DataBufferLLVM::CreateSliceFromPath(archive_file.GetPath(),
-                                                     512, file_offset);
+            data_sp = DataBufferLLVM::CreateSliceFromPath(
+                archive_file.GetPath(), 512, file_offset);
           }
         }
       }

                                            lldb::offset_t file_offset,
                                            lldb::offset_t file_size,
                                            ModuleSpecList &specs) {
-  DataBufferSP data_sp = DataBufferLLVM::CreateSliceFromPath(file.GetPath(), 512, file_offset);
+  DataBufferSP data_sp =
+      DataBufferLLVM::CreateSliceFromPath(file.GetPath(), 512, file_offset);
   if (data_sp) {
     if (file_size == 0) {
       const lldb::offset_t actual_file_size = file.GetByteSize();

 }
 
 Status ObjectFile::LoadInMemory(Target &target, bool set_pc) {
-  Status error;
   ProcessSP process = target.CalculateProcess();
   if (!process)
     return Status("No Process");

   SectionList *section_list = GetSectionList();
   if (!section_list)
     return Status("No section in object file");
+
+  Status error;
   size_t section_count = section_list->GetNumSections(0);
   for (size_t i = 0; i < section_count; ++i) {
     SectionSP section_sp = section_list->GetSectionAtIndex(i);

       section_sp->GetSectionData(section_data);
       lldb::offset_t written = process->WriteMemory(
           addr, section_data.GetDataStart(), section_data.GetByteSize(), error);
-      if (written != section_data.GetByteSize())
+      if (written != section_data.GetByteSize()) {
+        if (!error.Fail())
+          error.SetErrorStringWithFormat(
+              "One or more breakpoints intersect section '%s'",
+              section_sp->GetName().AsCString());
         return error;
+      }
     }
   }
   if (set_pc) {

   return error;
 }
 
-void ObjectFile::RelocateSection(lldb_private::Section *section)
-{
-}
+void ObjectFile::RelocateSection(lldb_private::Section *section) {}