[PATCH] D131631: [MSAN] Separate id ptr from constant string for variable names used in track origins.
Vitaly Buka via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 11 09:28:22 PDT 2022
vitalybuka added inline comments.
================
Comment at: compiler-rt/lib/msan/msan.cpp:315-318
+ if (id_ptr == nullptr) {
+ id_ptr = (u32 *)descr;
+ description = descr + 4;
+ }
----------------
we don't want a branch on fast path when we can do +4 in the caller
================
Comment at: compiler-rt/lib/msan/msan.cpp:615
void __msan_set_alloca_origin(void *a, uptr size, char *descr) {
- SetAllocaOrigin(a, size, descr,
+ SetAllocaOrigin(a, size, nullptr, descr,
StackTrace::GetPreviousInstructionPc(GET_CALLER_PC()));
----------------
================
Comment at: compiler-rt/lib/msan/msan.cpp:623
// older clang.
- SetAllocaOrigin(a, size, descr,
+ SetAllocaOrigin(a, size, nullptr, descr,
+ StackTrace::GetPreviousInstructionPc(GET_CALLER_PC()));
----------------
I guess existing code should work just fine
================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:239
cl::Hidden, cl::init(0xff));
static cl::opt<bool> ClPoisonUndef("msan-poison-undef",
----------------
kda wrote:
> vitalybuka wrote:
> > ```
> > static cl::opt<bool> ClPrintStacklNames("msan-print-stack-names",
> > cl::desc("Print name of local stack variable"),
> > cl::Hidden, cl::init(true));
> > ```
> What is this meant to do?
maybe don't add ClPrintStacklNames here
a follow up patch would be better
================
Comment at: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp:3904
+ Value *Descr = getLocalVarDescription(I);
IRB.CreateCall(MS.MsanSetAllocaOriginFn,
{IRB.CreatePointerCast(&I, IRB.getInt8PtrTy()), Len,
----------------
kda wrote:
> vitalybuka wrote:
> > ```
> > if (ClPrintLocalVNames) {
> > IRB.CreateCall(MS.MsanSetAllocaOriginNameFn, ...
> > } else {
> > IRB.CreateCall(MS.MsanSetAllocaOriginFn, ...
> > }
> > ```
> What is the goal here?
> Is this to allow the variable names to be dropped from the binary to save even more size?
yes
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131631/new/
https://reviews.llvm.org/D131631
More information about the llvm-commits
mailing list