[Lldb-commits] [PATCH] D18379: [JITLoaderGDB] Read jit entry struct manually.

[Greg Clayton via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

One thing to note: don't use Process::DoReadMemory(), it doesn't cache data and it will always result in a read memory packet going to GDB server if you are using lldb-server and you don't want that. Use Process::ReadMemory(), it will read stuff 512 bytes at a time and caching will work.

The main issue with this patch is your stuff won't work if you debug a program with different endianess that the host system. You need to read all the memory you need in a single memory read and then use a DataExtractor to extract stuff. DataExtractor gets constructed with a variety of data and also takes a byte order and an address byte size. The byte order will allow the extractor to byte swap the data correctly and the address byte size will allow you to extract an pointer using DataExtractor::GetPointer(...). The code for extractor should be much simpler than what you wrote above. Something like:

  const size_t addr_size = process->GetAddressByteSize();
  const size_t data_byte_size = addr_size * 3 + sizeof(uint64_t);
  DataBufferHeap data(data_byte_size, 0);
  size_t bytes_read = process->ReadMemory(from_addr, data.GetBytes(), data.GetByteSize(), error);
  if (bytes_read == data_byte_size)
  {
      DataExtractor extractor (data.GetBytes(), data.GetByteSize(), process->GetByteOrder(), addr_size);
      lldb::offset_t offset = 0;
      entry->next_entry = extractor.GetPointer(&offset);
      entry->prev_entry = extractor.GetPointer(&offset);
      entry->symfile_addr = extractor.GetPointer(&offset);
      entry->symfile_size = extractor.GetU64(&offset);
  }
  from_addr += offset;

Let me know if you have any questions.


================
Comment at: source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp:112-141
@@ +111,32 @@
+    {
+        addr_t addr = from_addr;
+        Error error;
+        size_t bytes_read = 0;
+
+         bytes_read = process->DoReadMemory(addr, &entry->next_entry, sizeof(ptr_t), error);
+         if (bytes_read != sizeof(ptr_t) || !error.Success())
+             return false;
+         addr += sizeof(ptr_t);
+
+         bytes_read = process->DoReadMemory(addr, &entry->prev_entry, sizeof(ptr_t), error);
+         if (bytes_read != sizeof(ptr_t) || !error.Success())
+             return false;
+         addr += sizeof(ptr_t);
+
+         bytes_read = process->DoReadMemory(addr, &entry->symfile_addr, sizeof(ptr_t), error);
+         if (bytes_read != sizeof(ptr_t) || !error.Success())
+             return false;
+         addr += sizeof(ptr_t);
+
+         ArchSpec::Core core = process->GetTarget().GetArchitecture().GetCore();
+         if (ArchSpec::kCore_x86_32_first <= core && core <= ArchSpec::kCore_x86_32_last)
+             addr += addr % 4;
+         else
+             addr += addr % 8;
+
+         bytes_read = process->DoReadMemory(addr, &entry->symfile_size, 8, error);
+         if (bytes_read != 8 || !error.Success())
+             return false;
+
+         return true;
+    }
----------------
There are many issues here: no byte swapping, multiple memory reads with no caching. See code in main comment for the simpler code to use.


http://reviews.llvm.org/D18379