ramosian-glider / sanitizer-issues

test
0 stars 0 forks source link

clang and clang++ behave differently on a small OOB test #28

Closed ramosian-glider closed 9 years ago

ramosian-glider commented 9 years ago

Originally reported on Google Code with ID 28

$ cat t.c
int volatile a[5];
int main() {
  a[5] = 1;
  return 0;
}

$ ../../../../build/Release+Asserts/bin/clang -faddress-sanitizer t.c -o t && ./t
<nothing here>

$ ../../../../build/Release+Asserts/bin/clang++ -faddress-sanitizer t.c -o t pp &&
./tpp

=================================================================
==44106== ERROR: AddressSanitizer global-buffer-overflow on address 0x000100015834
at pc 0x100007230 bp 0x7fff5fbff990 sp 0x7fff5fbff988
WRITE of size 4 at 0x000100015834 thread T0
    #0 0x100007230 (/Users/glider/src/asan/asan-llvm-trunk/llvm/projects/compiler-rt/lib/asan/./t+0x7230)
    #1 0x100007084 (/Users/glider/src/asan/asan-llvm-trunk/llvm/projects/compiler-rt/lib/asan/./t+0x7084)
    #2 0x1 (/Users/glider/src/asan/asan-llvm-trunk/llvm/projects/compiler-rt/lib/asan/./t+0x1)
0x000100015834 is located 0 bytes to the right of global variable 'a (t.c)' (0x100015820)
of size 20
  'a (t.c)' is ascii string ''
==44106== ABORTING
Stats: 0M malloced (0M for red zones) by 0 calls
Stats: 0M realloced by 0 calls
Stats: 0M freed by 0 calls
Stats: 0M really freed by 0 calls
Stats: 0M (0 full pages) mmaped in 0 calls
  mmaps   by size class: 
  mallocs by size class: 
  frees   by size class: 
  rfrees  by size class: 
Stats: malloc large: 0 small slow: 0
Shadow byte and word:
  0x100020002b06: 4
  0x100020002b00: 00 00 00 00 00 00 04 f9
More shadow bytes:
  0x100020002ae0: 00 00 00 00 00 00 00 00
  0x100020002ae8: 00 00 00 00 00 00 00 00
  0x100020002af0: 00 00 00 00 00 00 00 00
  0x100020002af8: 00 00 00 00 00 00 00 00
=>0x100020002b00: 00 00 00 00 00 00 04 f9
  0x100020002b08: f9 f9 f9 f9 00 00 00 00
  0x100020002b10: 00 00 00 00 00 00 00 00
  0x100020002b18: 00 00 00 00 00 00 00 00
  0x100020002b20: 00 00 00 00 00 00 00 00

Reported by ramosian.glider on 2012-01-17 12:16:22

ramosian-glider commented 9 years ago
There are some warnings about -Warray-bounds reported by both clang and clang++, but
passing -Wno-array-bounds does not fix the problem

Reported by ramosian.glider on 2012-01-17 12:24:12

ramosian-glider commented 9 years ago
$ ../../../../build/Release+Asserts/bin/clang  t.c -S -emit-llvm -o t.ll -O3
$ ../../../../build/Release+Asserts/bin/clang++  t.c -S -emit-llvm -o tpp.ll -O3
============================================================
$ diff t.ll tpp.ll
5c5
< @a = common global [5 x i32] zeroinitializer, align 16
---
> @a = global [5 x i32] zeroinitializer, align 16

============================================================
$ cat t.ll
; ModuleID = 't.c'
target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
target triple = "x86_64-apple-macosx10.6.8"

@a = common global [5 x i32] zeroinitializer, align 16

define i32 @main() nounwind uwtable ssp {
entry:
  store volatile i32 1, i32* getelementptr inbounds ([5 x i32]* @a, i64 1, i64 0),
align 4, !tbaa !0
  ret i32 0
}

!0 = metadata !{metadata !"int", metadata !1}
!1 = metadata !{metadata !"omnipotent char", metadata !2}
!2 = metadata !{metadata !"Simple C/C++ TBAA", null}
============================================================

$ cat tpp.ll
; ModuleID = 't.c'
target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
target triple = "x86_64-apple-macosx10.6.8"

@a = global [5 x i32] zeroinitializer, align 16

define i32 @main() nounwind uwtable ssp {
entry:
  store volatile i32 1, i32* getelementptr inbounds ([5 x i32]* @a, i64 1, i64 0),
align 4, !tbaa !0
  ret i32 0
}

!0 = metadata !{metadata !"int", metadata !1}
!1 = metadata !{metadata !"omnipotent char", metadata !2}
!2 = metadata !{metadata !"Simple C/C++ TBAA", null}

Reported by ramosian.glider on 2012-01-17 12:25:12

ramosian-glider commented 9 years ago
Looks like AddressSanitizer.cpp explicitly ignores globals with "common" linkage:

396     // Touch only those globals that will not be defined in other modules.
397     // Don't handle ODR type linkages since other modules may be built w/o asan.
398     if (G->getLinkage() != GlobalVariable::ExternalLinkage &&
399         G->getLinkage() != GlobalVariable::PrivateLinkage &&
400         G->getLinkage() != GlobalVariable::InternalLinkage)
401       continue;

Kostya, what will break down if we add CommonLinkage here?

As per http://llvm.org/docs/LangRef.html#linkage_common:

common
"common" linkage is most similar to "weak" linkage, but they are used for tentative
definitions in C, such as "int X;" at global scope. Symbols with "common" linkage are
merged in the same way as weak symbols, and they may not be deleted if unreferenced.
common symbols may not have an explicit section, must have a zero initializer, and
may not be marked 'constant'. Functions and aliases may not have common linkage.

Or maybe clang should not mark such global definitions as common?

Reported by ramosian.glider on 2012-01-17 12:48:46

ramosian-glider commented 9 years ago
We can not handle symbols with common linkage because such symbols 
may be redefined in another file to have a different size. 

% head a1.c* a2.c*
==> a1.c <==
int xxx;

==> a1.cc <==
int xxx;

==> a2.c <==
double xxx;
int main() { }

==> a2.cc <==
double xxx;
int main() { }
% clang a1.c a2.c
% clang++ a1.cc a2.cc
/tmp/a2-aqRav2.o:(.bss+0x0): multiple definition of `xxx'
/tmp/a1-7Snei5.o:(.bss+0x0): first defined here
/usr/bin/ld: Warning: size of symbol `xxx' changed from 4 in /tmp/a1-7Snei5.o to 8
in /tmp/a2-aqRav2.o
clang: error: linker command failed with exit code 1 (use -v to see invocation)
% 

Reported by konstantin.s.serebryany on 2012-01-17 18:13:10

ramosian-glider commented 9 years ago
Adding Project:AddressSanitizer as part of GitHub migration.

Reported by ramosian.glider on 2015-07-30 09:12:58