sivachandra added inline comments.
================
Comment at: source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp:110-119
@@ -80,3 +109,12 @@
-} // anonymous namespace end
+ template <typename T>
+ T
+ Align(const T v, uint8_t bytes)
+ {
+ uint8_t rem = v % bytes;
+ if (rem == 0)
+ return v;
+ else
+ return v + bytes - rem;
+ }
----------------
clayborg wrote:
> Unless this align is special and different from other align functions, we
> should use llvm::alignTo(...).
Removed.
================
Comment at: source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp:122
@@ +121,3 @@
+ template <typename ptr_t>
+ bool ReadJITEntry(const addr_t &from_addr, Process *process,
jit_code_entry<ptr_t> *entry)
+ {
----------------
clayborg wrote:
> Why is "from_addr" passed by reference? No one is updating this address and
> it is const. This should be "const addr_t from_addr" or it should be non
> const and "from_addr" should be updated in the call to ReadJITEntry().
Made it a value. Making it a reference was a slip.
================
Comment at: source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp:125-126
@@ +124,4 @@
+ ArchSpec::Core core = process->GetTarget().GetArchitecture().GetCore();
+ bool i386_target = ArchSpec::kCore_x86_32_first <= core && core <=
ArchSpec::kCore_x86_32_last;
+ uint8_t uint64_align_bytes = i386_target ? 4 : 8;
+ const size_t data_byte_size = Align(sizeof(ptr_t) * 3,
uint64_align_bytes) + sizeof(uint64_t);
----------------
clayborg wrote:
> Is this truly for i386 only? Not for all 32 bit architectures?
Per my reading of the arm32, mip32 and x86 (i386) ABI specs, only x86 aligns 8
byte values at 4-byte boundaries. Others align 8 byte values are 8-byte
boundaries.
For my testing, I have arm32 and i386 Android devices at hand which validate
the changes.
You can correct me if you think I got it wrong or if I have to consider more
archs.
================
Comment at: source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp:131
@@ +130,3 @@
+ DataBufferHeap data(data_byte_size, 0);
+ addr_t aligned_addr = Align(from_addr, sizeof(ptr_t)); // Playing
absolutely safe
+ size_t bytes_read = process->ReadMemory(aligned_addr, data.GetBytes(),
data.GetByteSize(), error);
----------------
clayborg wrote:
> Do you really want to adjust this here? Shouldn't this actually be:
>
> ```
> if (from_addr % sizeof(ptr_t))
> return false;
> ```
Replaced this with an assert.
http://reviews.llvm.org/D18379
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
