This is an archive of the discontinued LLVM Phabricator instance.

[lldb][PPC64] Fixed long double variables dump
ClosedPublic

Authored by luporl on Jan 15 2018, 11:04 AM.

Details

Summary

LLDB's DumpDataExtractor was not prepared to handle PowerPC's long double type: PPCDoubleDouble.

As it is somewhat special, treating it as other regular float types resulted in getting wrong information about it.
In this particular case, llvm::APFloat::getSizeInBits(PPCDoubleDouble) was returning 0.

This caused the TestSetValues.py test to fail, because lldb would abort on an assertion failure on APInt(), because of the invalid size.

This patch checks if the FloatTypeSemantics is PPCDoubleDouble and, if so, sets the byte size of the variable to 16.

This fixed the issue and the test is passing now.

Diff Detail

Repository
rL LLVM

Event Timeline

luporl created this revision.Jan 15 2018, 11:04 AM
luporl edited the summary of this revision. (Show Details)Jan 15 2018, 11:30 AM
luporl added reviewers: labath, clayborg.
luporl set the repository for this revision to rL LLVM.

This adds a special case to "long double" logic, which is already a special case compared to "float" and "double" cases. This was written this way (see http://reviews.llvm.org/D8417) because the x86 long double type is special, but if I understand this correctly, for the ppc type, the "default" logic of just using the item_byte_size would be correct.

What if we reverse the conditions here to be something like?

offset_t byte_size = item_byte_size;
if (&semantics == &x87DoubleExtended)
  byte_size = (APFloat::getSizeInBits(semantics)+7)/8;

@labath, the change you suggested really looks better, by reducing the number of special cases.

I tested it on PPC64 and it also fixes the issue, as expected.

I'll update the diff.
Thanks!

luporl updated this revision to Diff 129939.Jan 16 2018, 4:38 AM

Changed implementation to make only x87DoubleExtended a special case.

labath accepted this revision.Jan 17 2018, 3:09 AM

Awesome, thanks.

This revision is now accepted and ready to land.Jan 17 2018, 3:09 AM

@labath, can you please commit the changes for me? (I don't have permission to do it)

This revision was automatically updated to reflect the committed changes.