This is an archive of the discontinued LLVM Phabricator instance.

Append line number to variable name if line is available and in the same file as the function.
ClosedPublic

Authored by vitalybuka on Oct 18 2016, 12:24 AM.

Event Timeline

vitalybuka retitled this revision from to Append line number to variable name if line is available and in the same file as the function..
vitalybuka updated this object.
vitalybuka added a reviewer: eugenis.
eugenis added inline comments.Oct 18 2016, 10:56 AM
lib/Transforms/Instrumentation/AddressSanitizer.cpp
2262

while technically correct, "const unsigned" variables don't seem common in llvm. Remove const?

2264

I don't get this line at all. What's the meaning of the smallest of two line numbers?
Why do you store the line number in two places (inside the struct and as the second element of the pair?

vitalybuka marked an inline comment as done.

remove const

vitalybuka marked an inline comment as done.Oct 18 2016, 11:53 AM
vitalybuka added inline comments.
lib/Transforms/Instrumentation/AddressSanitizer.cpp
2264

ASanStackVariableDescription is for allocas
But line information only available from lifteme intrinsics, and we can have several of them. I assume that smallest is where var was declared.

eugenis added inline comments.Oct 18 2016, 1:10 PM
lib/Transforms/Instrumentation/AddressSanitizer.cpp
2260

Why reset Pair.first?
Can it be replaced with assert(!Pair.first) ?

2264

OK. AILoc is a bad name, because it is not debugloc of AI, it's debugloc of the lifetime intrinstic.

Can this whole loop be moved below, after ASanStackVariableDescription objects are created, and then use Line field in the struct directly?

vitalybuka updated this revision to Diff 75070.Oct 18 2016, 2:03 PM
vitalybuka marked 4 inline comments as done.

update

vitalybuka added inline comments.Oct 18 2016, 2:09 PM
lib/Transforms/Instrumentation/AddressSanitizer.cpp
2264

Can this whole loop be moved below, after ASanStackVariableDescription objects are created, and then use Line field in the struct directly?

It can't without changes in ComputeASanStackFrameLayout in ASanStackFrameLayout
It both sorts and builds stack description ASanStackVariableDescription and needs sizes and lines provided

eugenis accepted this revision.Oct 18 2016, 3:07 PM
eugenis edited edge metadata.

D25754 answers all my questions and makes this LGTM

This revision is now accepted and ready to land.Oct 18 2016, 3:07 PM
This revision was automatically updated to reflect the committed changes.

This broke android builds because it uses std::to_string instead of llvm::to_string. Would you mind changing it? https://reviews.llvm.org/D25754 adds another use of std::to_string. I would fix it but don't have commit access. Thanks!