[PATCH] D123010: [asan] Emit .size directive for global object size before redzone

[Alex Brachet via Phabricator via llvm-commits]

abrachet marked 5 inline comments as done.
abrachet added inline comments.


================
Comment at: llvm/docs/LangRef.rst:7098
+
+The ``explicit_size`` metadata is used to emit a size directive with a different
+size than the objects total size. This can be useful when an object contains
----------------
efriedma wrote:
> MaskRay wrote:
> > Suggest: The ``explicit_size`` metadata  may be attached to a global variable definition with a size different from the object's total size. This can be useful when an instrumentation enlarges the object while the symbol size should reflect the accessible or meaningful part of the object.
> > 
> > 
> > I think this is not meaningful to functions, ifuncs, comdats, etc, and likely not meaningful to scalar types, but it may not be necessary to catch the error in the IR verifier.
> Probably makes sense to diagnose on anything that isn't a definition of a variable.  Probably not worth trying to catch other dubious cases.
> 
> Maybe explicitly note that this doesn't make sense for all object formats?
Thanks @maskray I've used this comment verbatim. Plus added some about how this is only meaningful on ELF. 

> Probably makes sense to diagnose on anything that isn't a definition of a variable.
How do you reckon I would do this? It seems like the current behavior is to just ignore metadata that doesn't get used. Seems difficult to warn about places where `!explicit_size` isn't meaningful. Do you know of an example where this happens in llvm currently I could look at?


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:694
+  if (MAI->hasDotTypeDotSizeDirective()) {
+    [&Size, GV] {
+      const MDNode *ExplicitValue = GV->getMetadata("explicit_size");
----------------
MaskRay wrote:
> Such IIFE is not common. Why not just remove the lambda?
Sure. I was thinking it would be too many nested if's. But I guess it looks fine.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123010/new/

https://reviews.llvm.org/D123010