[PATCH] D53227: [hwasan] add stack frame descriptions.

[Kostya Serebryany via Phabricator via llvm-commits]

kcc added inline comments.


================
Comment at: lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:151
+static cl::opt<bool>
+    ClCreateFrameDescriptions("hwasan_create_frame_descriptions",
+                              cl::desc("create static frame descriptions"),
----------------
eugenis wrote:
> s/_/-
done. 


================
Comment at: lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:217
+  // Get the section name for FD. Currently ELF-only.
+  const char *getFDSection() { return "__hwasan_fd"; }
+  const char *getFDSectionBeg() { return  "__start___hwasan_fd"; }
----------------
eugenis wrote:
> "fd" usually stands for file descriptor
> Rename to __hwasan_frames?
renamed the section to __hwasan_frames and the callback to __hwasan_init_frames


================
Comment at: lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:319
+    // * w/o it we would have to create the call to __hwasan_init_fd after
+    //   all functions are instrumented (i.e. need to have a ModulePass).
+    createFD(*HwasanCtorFunction, "");
----------------
eugenis wrote:
> This section should be read-only so the (intrusive) linked list idea would not work.
> 
> I don't think this slot helps you avoid a module pass. You can create a call to __hwasan_init_fd regardless. 
> 
> You are inserting multiple calls to __hwasan_init_fd to the ctor function. Just one would be enough.
> 
> Ctor function should be comdat'ed itself, but that's orthogonal.
Ok, won't use the (intrusive) linked list. 

> You are inserting multiple calls to __hwasan_init_fd to the ctor function. 

Nope. This code is in HWAddressSanitizer::doInitialization(Module &M)

> Ctor function should be comdat'ed itself, but that's orthogonal.

Yep, will do separately. 



================
Comment at: lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:742
+  // Put GV into the F's Comadat so that if F is deleted GV can be deleted too.
+  if (!F.getName().startswith("hwasan"))
+    if (auto Comdat = GetOrCreateFunctionComdat(F, CurModuleUniqueId))
----------------
eugenis wrote:
> Why would F's name start with "hwasan"? Did you mean "__hwasan"?
No, this one is for the CTOR (HwasanCtorFunction above)


Repository:
  rL LLVM

https://reviews.llvm.org/D53227