[DebugInfo] Correction for an assert in DIExpression::createFragmentExpression

[bjope]

Summary

When we create a fragment expression, and there already is an
old fragment expression, we assert that the new fragment is
within the range for the old fragment.

If for example the old fragment expression says that we
describe bit 10-16 of a variable (Offset=10, Size=6),
and we now want to create a new fragment expression only
describing bit 3-6 of the original value, then the resulting
fragment expression should have Offset=13, Size=3.

The assert is supposed to catch if the resulting fragment
expression is outside the range for the old fragment. However,
it used to verify that the Offset+Size of the new fragment was
smaller or equal than Offset+Size for the old fragment. What
we really want to check is that Offset+Size of the new fragment
is smaller than the Size of the old fragment.

[davide]

Does this fire anywhere? If so, we should try adding a test.

[bjope]

In D46391#1086470, @davide wrote:

Does this fire anywhere? If so, we should try adding a test.

We did hit the assert in PR37321 (but with Op.getArg(0)==0), that problem will be solved in D46384.
The incorrect upper limit for Op.getArg(0)!=0 was more or less spotted by code inspection.

[aprantl]

So this is making the assertion even stricter. Thanks!

lib/IR/DebugInfoMetadata.cpp

832 ↗

(On Diff #145018)

Actually this comment is silly, too:

uint64_t FragmentOffsetInBits = Op.getArg(0);
uint64_t FragmentSizeInBits = Op.getArg(1);
(void)FragmentSizeInBits;
assert((OffsetInBits + SizeInBits <= FragmentSizeInBits) && ....);

[bjope]

lib/IR/DebugInfoMetadata.cpp

832 ↗

(On Diff #145018)

Ok, I'll fix that as well. Thanks for the review!

llvm/trunk/lib/IR/DebugInfoMetadata.cpp

for (auto Op : Expr->expr_ops()) {
         // can't express carry-over between fragments.
         //
         // FIXME: We *could* preserve the lowest fragment of a constant offset
         // operation if the offset fits into SizeInBits.
         return None;
       case dwarf::DW_OP_LLVM_fragment: {
         // Make the new offset point into the existing fragment.
         uint64_t FragmentOffsetInBits = Op.getArg(0);
-        // Op.getArg(0) is FragmentOffsetInBits.
-        // Op.getArg(1) is FragmentSizeInBits.
-        assert((OffsetInBits + SizeInBits <= Op.getArg(0) + Op.getArg(1)) &&
+        uint64_t FragmentSizeInBits = Op.getArg(1);
+        (void)FragmentSizeInBits;
+        assert((OffsetInBits + SizeInBits <= FragmentSizeInBits) &&
                "new fragment outside of original fragment");
         OffsetInBits += FragmentOffsetInBits;
         continue;
       }
       }
       Ops.push_back(Op.getOp());
       for (unsigned I = 0; I < Op.getNumArgs(); ++I)
         Ops.push_back(Op.getArg(I));