[sanitizer] Caught global buffer underflow for first variable

[condy]

Summary

There is no left redzone for global variables, so the underflow of the first variable couldn't be caught.
This patch creates a zero-sized array before the first variable so the underflow of it could be observable.

[condy]

Fix some tests

[condy]

Fix some tests

[condy]

Fix all tests

[condy]

Fix typos

[vitalybuka]

Would you mind to add compiler-rt/test/asan which triggers this one?

[eugenis]

You are adding a new global to every translation unit.

• Private linkage would not allow them to be merged, this can have significant binary size and RAM overhead.
• There is no guarantee that any of these globals will end up to the left of any sanitized globals. With -fdata-sections, the linker is free to reorder.
• It is also not referenced from anything, so -gc-sections is likely to kill it.

It looks like this will only work in very limited circumstances.

[condy]

In D102592#2764085, @vitalybuka wrote:

Would you mind to add compiler-rt/test/asan which triggers this one?

Yes, I will.

[condy]

In D102592#2764357, @eugenis wrote:

You are adding a new global to every translation unit.

• Private linkage would not allow them to be merged, this can have significant binary size and RAM overhead.

Do you suggest linkonce linkage?

• There is no guarantee that any of these globals will end up to the left of any sanitized globals. With -fdata-sections, the linker is free to reorder.
• It is also not referenced from anything, so -gc-sections is likely to kill it.

It looks like this will only work in very limited circumstances.

Since it's limited, perhaps making it opt-in is a better choice. -asan-global-underflow or something else.

[eugenis]

I think it is too hit-and-miss to add even under a flag. It's just not the right approach - but I also don't know what the right approach would be.

Perhaps adding a small left redzone for all globals and reducing the right redzone to keep the total size under control? This way when most globals are instrumented we get approximately the same amount of redzone between any 2 of them, but also a little on the left of the first one.

[condy]

In D102592#2769811, @eugenis wrote:

I think it is too hit-and-miss to add even under a flag. It's just not the right approach - but I also don't know what the right approach would be.

Perhaps adding a small left redzone for all globals and reducing the right redzone to keep the total size under control? This way when most globals are instrumented we get approximately the same amount of redzone between any 2 of them, but also a little on the left of the first one.

Thanks for explanation, I will try your suggested approach.

[vitalybuka]

So I will hide it from "Ready to Review" list.

clang/test/CodeGen/asan-globals-alias.cpp

 
 // Check the aliases exist:
 // CHECK: @__global_alias ={{.*}} alias
 // CHECK: @global_alias_2 ={{.*}} alias
 // CHECK: @__global_alias_2_alias ={{.*}} alias
 // CHECK: @__mod_joydev_ids_device_table ={{.*}} alias
 
 // CHECK-LABEL: define internal void @asan.module_ctor
-// ASAN: call void @__asan_register_globals({{.*}}, i{{32|64}} 4)
-// KASAN: call void @__asan_register_globals({{.*}}, i{{32|64}} 1)
+// ASAN: call void @__asan_register_globals({{.*}}, i{{32|64}} 5)
+// KASAN: call void @__asan_register_globals({{.*}}, i{{32|64}} 2)
 // CHECK-NEXT: ret void
 
 // CHECK-LABEL: define internal void @asan.module_dtor
 // CHECK-NEXT: call void @__asan_unregister_globals
 // CHECK-NEXT: ret void

clang/test/CodeGen/asan-globals-odr.cpp

 // INDICATOR0-NOT: __odr_asan_gen
 // INDICATOR1: [[ODR:@.*__odr_asan_gen_.*global.*]] = global i8 0, align 1
 
 // GLOB_VAR: @0 = internal global {{.*}} [[VAR]] to i64), {{.*}}, i64 0 }]
 // GLOB_VAR_INDICATOR: @0 = internal global {{.*}} [[VAR]] to i64), {{.*}}, i64 ptrtoint (i8* [[ODR]] to i64) }]
 // GLOB_ALIAS_INDICATOR: @0 = internal global {{.*}} @1 to i64), {{.*}}, i64 ptrtoint (i8* [[ODR]] to i64) }]
 
 // ALIAS0-NOT: private alias
-// ALIAS1: @1 = private alias {{.*}} [[VAR]]
+// ALIAS1: @2 = private alias {{.*}} [[VAR]]
 
-// CHECK: call void @__asan_register_globals(i64 ptrtoint ([1 x { i64, i64, i64, i64, i64, i64, i64, i64 }]* @0 to i64), i64 1)
-// CHECK: call void @__asan_unregister_globals(i64 ptrtoint ([1 x { i64, i64, i64, i64, i64, i64, i64, i64 }]* @0 to i64), i64 1)
+// CHECK: call void @__asan_register_globals(i64 ptrtoint ([2 x { i64, i64, i64, i64, i64, i64, i64, i64 }]* @0 to i64), i64 2)
+// CHECK: call void @__asan_unregister_globals(i64 ptrtoint ([2 x { i64, i64, i64, i64, i64, i64, i64, i64 }]* @0 to i64), i64 2)

clang/test/CodeGen/asan-globals.cpp

 // KASAN: @__special_global{{.*}} global i32
 
 /// Without -fasynchronous-unwind-tables, ctor and dtor get the uwtable attribute.
 // CHECK-LABEL: define internal void @asan.module_ctor() #[[#ATTR:]] {
 // ASAN-NEXT: call void @__asan_init
 // ASAN-NEXT: call void @__asan_version_mismatch_check
 // KASAN-NOT: call void @__asan_init
 // KASAN-NOT: call void @__asan_version_mismatch_check
-// ASAN-NEXT: call void @__asan_register_globals({{.*}}, i{{32|64}} 7)
-// KASAN-NEXT: call void @__asan_register_globals({{.*}}, i{{32|64}} 5)
+// ASAN-NEXT: call void @__asan_register_globals({{.*}}, i{{32|64}} 8)
+// KASAN-NEXT: call void @__asan_register_globals({{.*}}, i{{32|64}} 6)
 // CHECK-NEXT: ret void
 
 // CHECK:      define internal void @asan.module_dtor() #[[#ATTR]] {
 // CHECK-NEXT: call void @__asan_unregister_globals
 // CHECK-NEXT: ret void
 
 // CHECK: attributes #[[#ATTR]] = { nounwind }
 

clang/test/CodeGen/asan-static-odr.cpp

 // RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - -triple x86_64-linux %s | FileCheck %s
 
 // No alias on Windows but indicators should work.
 // RUN: %clang_cc1 -fsanitize=address -emit-llvm -o - -triple x86_64-windows-msvc %s | FileCheck %s
 
 static int global;
 
 int main() {
   return global;
 }
 
 // CHECK-NOT: __odr_asan_gen
 // CHECK-NOT: private alias
 // CHECK: [[VAR:@.*global.*]] ={{.*}} global { i32, [60 x i8] } zeroinitializer, align 32
 // CHECK: @0 = internal global {{.*}} [[VAR]] to i64), {{.*}}, i64 -1 }]
-// CHECK: call void @__asan_register_globals(i64 ptrtoint ([1 x { i64, i64, i64, i64, i64, i64, i64, i64 }]* @0 to i64), i64 1)
-// CHECK: call void @__asan_unregister_globals(i64 ptrtoint ([1 x { i64, i64, i64, i64, i64, i64, i64, i64 }]* @0 to i64), i64 1)
+// CHECK: call void @__asan_register_globals(i64 ptrtoint ([2 x { i64, i64, i64, i64, i64, i64, i64, i64 }]* @0 to i64), i64 2)
+// CHECK: call void @__asan_unregister_globals(i64 ptrtoint ([2 x { i64, i64, i64, i64, i64, i64, i64, i64 }]* @0 to i64), i64 2)

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

bool ModuleAddressSanitizer::InstrumentGlobals(IRBuilder<> &IRB, Module &M,
   }
 
   size_t n = GlobalsToChange.size();
   if (n == 0) {
     *CtorComdat = true;
     return false;
   }
 
+  // Create a global zero-sized array precedence before GlobalsToChange[0] so
+  // that the underflow of the it could be observable.
+  ArrayType *ZeroSizedArrayTy = ArrayType::get(IRB.getInt8Ty(), 0);
+  GlobalVariable *UnderflowObserverGV = new GlobalVariable(
+      M, ZeroSizedArrayTy, /*isConstant*/ false, GlobalVariable::PrivateLinkage,
+      ConstantArray::get(ZeroSizedArrayTy, {}), kAsanGenPrefix,
+      GlobalsToChange[0]);
+  GlobalsToChange.insert(GlobalsToChange.begin(), UnderflowObserverGV);
+
+  // Update size as we insert a dummy one.
+  n = GlobalsToChange.size();
+
   auto &DL = M.getDataLayout();
 
   // A global is described by a structure
   //   size_t beg;
   //   size_t size;
   //   size_t size_with_redzone;
   //   const char *name;
   //   const char *module_name;

llvm/test/Instrumentation/AddressSanitizer/global_metadata.ll

 ; CHECK: @.str = internal constant { [14 x i8], [50 x i8] } { [14 x i8] c"Hello, world!\00", [50 x i8] zeroinitializer }, align 32
 
 ; Check emitted location descriptions:
 ; CHECK: [[VARNAME:@___asan_gen_.[0-9]+]] = private unnamed_addr constant [7 x i8] c"global\00", align 1
 ; CHECK: [[FILENAME:@___asan_gen_.[0-9]+]] = private unnamed_addr constant [22 x i8] c"/tmp/asan-globals.cpp\00", align 1
 ; CHECK: [[LOCDESCR:@___asan_gen_.[0-9]+]] = private unnamed_addr constant { [22 x i8]*, i32, i32 } { [22 x i8]* [[FILENAME]], i32 5, i32 5 }
 ; NOALIAS: @__asan_global_global = {{.*}}i64 ptrtoint ({ i32, [60 x i8] }* @global to i64){{.*}} section "asan_globals"{{.*}}, !associated
 ; NOALIAS: @__asan_global_.str = {{.*}}i64 ptrtoint ({ [14 x i8], [50 x i8] }* @{{.str|1}} to i64){{.*}} section "asan_globals"{{.*}}, !associated
-; ALIAS: @__asan_global_global = {{.*}}i64 ptrtoint ({ i32, [60 x i8] }* @0 to i64){{.*}} section "asan_globals"{{.*}}, !associated
-; ALIAS: @__asan_global_.str = {{.*}}i64 ptrtoint ({ [14 x i8], [50 x i8] }* @3 to i64){{.*}} section "asan_globals"{{.*}}, !associated
+; ALIAS: @__asan_global_global = {{.*}}i64 ptrtoint ({ i32, [60 x i8] }* @1 to i64){{.*}} section "asan_globals"{{.*}}, !associated
+; ALIAS: @__asan_global_.str = {{.*}}i64 ptrtoint ({ [14 x i8], [50 x i8] }* @4 to i64){{.*}} section "asan_globals"{{.*}}, !associated
 
 ; The metadata has to be inserted to llvm.compiler.used to avoid being stripped
 ; during LTO.
 ; CHECK: @llvm.compiler.used {{.*}} @__asan_global_global {{.*}} section "llvm.metadata"
 
 ; Check that location descriptors and global names were passed into __asan_register_globals:
 ; CHECK: call void @__asan_register_elf_globals(i64 ptrtoint (i64* @___asan_globals_registered to i64), i64 ptrtoint (i64* @__start_asan_globals to i64), i64 ptrtoint (i64* @__stop_asan_globals to i64))