This is an archive of the discontinued LLVM Phabricator instance.

Provide a method for generating deterministic IDs for pointers allocated in BumpPtrAllocator
ClosedPublic

Authored by george.karpenkov on Aug 28 2018, 5:21 PM.

Details

Summary

There are many "dumping" actions available from the compiler: Clang dumps AST, Clang static analyzer dumps generated nodes in the "exploded graph" it generates.

However, in many of those dumps raw pointer values are printed for the objects of interest, which can considerably complicate debugging due to those values being non-reproducible.
This patch adds a method for converting a pointer generated through an allocator to a deterministic (up to an architecture) long, which can make the debugging experience much more pleasant.
(E.g. compare hunting for a value "0xa9273f732" which changes every run to hunting for "23432").

Discussion started at http://lists.llvm.org/pipermail/cfe-dev/2018-August/059178.html

Code in collaboration with @NoQ

Diff Detail

Event Timeline

aprantl added inline comments.Aug 29 2018, 8:39 AM
llvm/include/llvm/Support/Allocator.h
292

clang-format?

295

return {Idx, P - S};

Just for consideration: The raw pointers in dumps are sometimes useful while in a debugger session, because you can cast a pointer and dump the object in the debugger.

NoQ added a comment.Aug 29 2018, 2:09 PM

Just for consideration: The raw pointers in dumps are sometimes useful while in a debugger session, because you can cast a pointer and dump the object in the debugger.

Yup, i use that as well from time to time, so i guess we can either dump both or make a flag.

llvm/include/llvm/Support/Allocator.h
290

I'd much rather have the first element signed, because it's the only one that can actually carry "negative" values. Maybe just let's do two int64_ts or intptr_ts?

295

And, well, let's make integral casts explicit here.

george.karpenkov edited the summary of this revision. (Show Details)

In fact, generating a single "long" seems even better.

NoQ added a comment.Aug 29 2018, 4:16 PM

In fact, generating a single "long" seems even better.

ptrdiff_t seems to express what we're trying to say pretty accurately.

NoQ added inline comments.Sep 4 2018, 12:43 PM
llvm/include/llvm/Support/Allocator.h
304

We should start with -1 because otherwise the first standard-slab object and the first custom-slab object would both have id 0.

xbolva00 added inline comments.
llvm/include/llvm/Support/Allocator.h
295

for (size_t Idx = 0, e = Slabs.size(); Idx < e; Idx++) {

george.karpenkov marked 2 inline comments as done.
george.karpenkov updated this revision to Diff 164292.
NoQ accepted this revision.Sep 6 2018, 3:17 PM

I'm happy with the code.

llvm/include/llvm/Support/Allocator.h
298

Two spaces here.

This revision is now accepted and ready to land.Sep 6 2018, 3:17 PM
This revision was automatically updated to reflect the committed changes.