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

[Slava Gurevich via Phabricator via lldb-commits]

fixathon marked 2 inline comments as done.
fixathon added inline comments.


================
Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:540
+        for (uint32_t i = 0;
+             count > 0 && count <= sizeof(gpr.r) && i < count - 1; ++i) {
           gpr.r[i] = data.GetU32(&offset);
----------------
fixathon wrote:
> jasonmolenda wrote:
> > jasonmolenda wrote:
> > > clayborg wrote:
> > > > 
> > > The `count` field for a Darwin register context is the number of 4-byte entries in the object - it's a trick the kernel API often use so they can add fields later and the kernel knows what version of the object the userland process is requesting when it asks for "flavor, size" in a `get_thread_state` call.  This Aarch32 register context is `struct GPR {uint32_t r[16]; uint32_t cpsr};` or count 17, but `sizeof(gpr.r)` is going to be 64.  We only want to loop for 16 entries.
> > FWIW the Aarch64 version of this function hardcodes the number of elements (where each general purpose register is 8-bytes, so count==2, and then there's one cpsr 4-byte register),
> > ```
> >         // x0-x29 + fp + lr + sp + pc (== 33 64-bit registers) plus cpsr (1
> >         // 32-bit register)
> >         if (count >= (33 * 2) + 1) {
> >           for (uint32_t i = 0; i < 29; ++i)
> >             gpr.x[i] = data.GetU64(&offset);
> >           gpr.fp = data.GetU64(&offset);
> >           gpr.lr = data.GetU64(&offset);
> >           gpr.sp = data.GetU64(&offset);
> >           gpr.pc = data.GetU64(&offset);
> >           gpr.cpsr = data.GetU32(&offset);
> > ```
> Good catch. I'll: count <= sizeof(gpr.r)/sizeof(gpr.r[0]) 
"count > 0 && count <= sizeof(gpr.r)/sizeof(uint32_t) && i < count - 1; ++i)" was causing a compiler warning, which is the reason I moved **count** out of the for loop condition


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131554/new/

https://reviews.llvm.org/D131554