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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:697
+             "Should have been checked by verifier allready");
+      const auto *MetadataValue =
+          dyn_cast<ValueAsMetadata>(ExplicitValue->getOperand(0));
----------------
Use `cast` which asserts non-null.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:701
+      const auto *CI = dyn_cast<ConstantInt>(MetadataValue->getValue());
+      assert(CI && "Should have been checked by verifier allready");
+      Size = CI->getZExtValue();
----------------
Use `cast` which asserts non-null.


================
Comment at: llvm/test/CodeGen/X86/explicit-size-metadata.ll:1
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu | FileCheck %s
+
----------------
Try making tests more general by checking the generic ELF behavior, not linux-gnu specific ones.


================
Comment at: llvm/test/Instrumentation/AddressSanitizer/adaptive_global_redzones.ll:9
 @G10 = global [10 x i8] zeroinitializer, align 1
-; CHECK: @G10 = global { [10 x i8], [22 x i8] }
+; CHECK: @G10 = global { [10 x i8], [22 x i8] } zeroinitializer, align 32, !explicit_size ![[TAG10:.*]]
 
----------------
`![[#TAG10:]]` (match a decimal)

Reference it below with `[[#TAG10]]`

This is better than `.*` which matches too much, and `{{[0-9]+}}` which is too verbose.


================
Comment at: llvm/test/Instrumentation/AddressSanitizer/adaptive_global_redzones.ll:56
 @G100000000 = global [100000000 x i8] zeroinitializer, align 1
 ; CHECK: @G1000000 = global { [1000000 x i8], [249984 x i8] }
 ; CHECK: @G10000000 = global { [10000000 x i8], [262144 x i8] }
----------------
MaskRay wrote:
> Update the two CHECK lines as well.
`![[#]]`


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

https://reviews.llvm.org/D123010