Thanks everyone, I'll land this shortly.

Thanks @vsk.

Makes sense to separate out these parts. LGTM, but it's a sizeable patch so maybe wait a little and see if anyone else has anything to say before committing.

LGTM

LGTM but since we're on the same team (now, woot!) I'd like to give others a chance to have a look.

LGTM.

I like the idea of this and think it looks useful, and personally I would say
please do continue to work on it after giving people a chance to reply to this
comment.

Adding Stephen and Tom who have also worked on dexter.

LGTM. Thanks for the patch, It's great to see someone using and contributing to dexter!

LGTM

LGTM

Thanks for reviewing these patches! I will land them now.

Address review comments:
+ match() -> matches()

+ Rebase

+ Rebase (see 6531a78ac4b5 caused conflicts) and remove parent.
+ Include bugzilla links in summary.

+ Rebase
+ Rename wasIndirect() to getWasIndirect() to match the other changes in this patch.

Thanks for reviewing this. Unfortunately the changes made in ed29dbaafa49 (D68945) were just reverted in 6531a78ac4b5 so this patch is no longer necessary.

Thanks for reviewing this @djtodoro

Thanks for looking at these patches. When they are all ready to land would it be better to land them independently or all together as one?

Address comments:

I've opened a stack of patches starting at D74052 which implement the 99.99...% fix (handle fully overlapping fragments) discussed above.

Orlando has looked at this more recently and is probably more qualified to review ;)

Let me see if I can restate that correctly. So, the problem here is: for the list of user-values tracked by the interval map for the range containing [DV1, DV3], there are two entries: one for {DV1, DV3} (coalesced), and another for {DV2}. This would result in "x" being interpreted as %1 + 4 instead of %0 in that interval (by the debugger)?

@vsk this is the situation as far as I understand it. I appreciate that this is
a lot of text; there is a summary at the end. I hope this helps.

Hi @vsk, thank you for taking a look at this. Please bear with me, getting you some (relatively concise) answers it is taking a little longer than I expected but I am working on it.

Hi @djtodoro. I will create new patches.

So how did you come to be trying to fix these bugs?

I was specifically looking for cases where debug intrinsics are dropped.

Hi @dblaikie, thanks for your comments.

See D73478

See D73478

See D73478.

Ok no problem, will do. I'll abandon this, D73473, D73479, and D73481 and combine all the patches which touch the same pass.

Ping. Please see my previous comment for an update on this.

Thanks @djtodoro for looking at this. Sorry for this really delayed response!

Hi, sorry for the slow update.

As far as I understand it I don't think it is safe to move CFI_INSTRUCTIONs
around like this. CFI_INSTRUCTIONs describe the effect of specific instructions.
We encode where this 'description' starts by the position of the CFI_INSTRUCTION
in the instruction stream. So, if you move the CFI_INSTRUCTION like this you
are changing the meaning.

Thanks for taking a look @aprantl.

Missed one of djtodoro's comments, sorry for the spam!

+ Rebase.
+ Address comments from @djtodoro.

+ Update dexter test for @jmorse.
+ Small additional change to print().
+ Rebase.
+ Rephrase a small part of the summary.

Adrian said:

Thanks! Let's cycle back to this after the llvm-dev thread that @dblaikie started has been resolved.

Thanks for the review, committed in 2de23c8364b.

Hi @alok. I haven't been following the DW_OP_implicit_pointer discussion too closely, so just some inline nitpick comments/questions from me.

LGTM -- though having given only one LGTM before I'd feel more comfortable waiting for +1.

@aprantl wrote https://reviews.llvm.org/D70318#inline-635016:

Thanks all for the initial reviews.

+ Added reviewers

+ Improve some comments
+ rename getter/setters as Adrian requested
+ remove some whitespace changes