Page MenuHomePhabricator

[LLDB][MIPS] Fix ReadRegisterValue for registers with constant 32 bit size regardless of ABI
ClosedPublic

Authored by mohit.bhakkad on Jan 11 2016, 5:16 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

mohit.bhakkad retitled this revision from to [LLDB][MIPS] Fix ReadRegisterValue for registers with constant 32 bit size regardless of ABI.
mohit.bhakkad updated this object.
mohit.bhakkad set the repository for this revision to rL LLVM.
tberghammer requested changes to this revision.Jan 11 2016, 5:37 AM
tberghammer edited edge metadata.

Please see inline comments

P.S.: Next time please upload the diff with full context (git diff -U999999999) as it is easier to review that way

source/Plugins/Process/Linux/NativeRegisterContextLinux_mips64.cpp
1392 ↗(On Diff #44467)

You can't compare a string literal and a const char* like this (it will compare the pointers only). You have to use ::strcmp or use the ConstString class in some form.

1393–1395 ↗(On Diff #44467)

Reading these lines are very difficult. I think something like this would be much cleaner (I haven't fixed the string comparisons and haven't tested it):

void* target_address = ((uint8_t*)&regs) + offset + 4 * (arch.GetMachine() == llvm::Triple::mips);
uint32_t target_size;
if (reg_name == "sr" || reg_name == "cause" || reg_name == "config5")
    target_size = 4;
else
    target_size = arch.GetFlags() & lldb_private::ArchSpec::eMIPSABI_O32 ? 4 : 8;
value.SetBytes(target_address, target_size, arch.GetByteOrder());
This revision now requires changes to proceed.Jan 11 2016, 5:38 AM
mohit.bhakkad edited edge metadata.

Sorry for the mess in last revision.
Updated as per suggestion.

tberghammer accepted this revision.Jan 11 2016, 8:19 AM
tberghammer edited edge metadata.

Looks good. Thanks for cleaning it up

This revision is now accepted and ready to land.Jan 11 2016, 8:19 AM
clayborg accepted this revision.Jan 11 2016, 10:57 AM
clayborg edited edge metadata.
mohit.bhakkad edited edge metadata.Jan 11 2016, 9:58 PM
mohit.bhakkad added a subscriber: lldb-commits.
This revision was automatically updated to reflect the committed changes.