Incorrect shadow values for global string constants on OSX

[GoogleCodeExporter]

See http://crbug.com/352073

=================================================================
==38424==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000f8362 
at pc 0x14b16d bp 0xbff7f5f8 sp 0xbff7f5e8
READ of size 1 at 0x000f8362 thread T0
    #0 0x14b16c in wrap_memmove (/Volumes/data/b/build/slave/mac_asan/build/src/third_party/llvm-build/Release+Asserts/lib/clang/3.5/lib/darwin/libclang_rt.asan_osx_dynamic.dylib+0x1716c)
    #1 0x965fe351 in __CFStringAppendBytes (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation+0x8351)
    #2 0x965fd99e in __CFStringAppendFormatCore (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation+0x799e)
    #3 0x9664a19b in _CFStringCreateWithFormatAndArgumentsAux (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation+0x5419b)
    #4 0x9575beed in -[NSPlaceholderString initWithFormat:locale:arguments:] (/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation+0x5beed)
    #5 0x9575d04b in +[NSString stringWithFormat:] (/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation+0x5d04b)
    #6 0x80ab8 in main (/Volumes/data/b/build/slave/mac_asan/build/src/chrome/../out/Release/infoplist_strings_tool+0x2ab8)
    #7 0x80254 in start (/Volumes/data/b/build/slave/mac_asan/build/src/chrome/../out/Release/infoplist_strings_tool+0x2254)

0x000f8362 is located 2 bytes inside of global variable '.str119' from 
'../../chrome/tools/mac_helpers/infoplist_strings_util.mm' (0xf8360) of size 12
  '.str119' is ascii string '%d.%d.%d.%d'
0x000f8362 is located 27 bytes to the right of global variable '.str117' from 
'../../chrome/tools/mac_helpers/infoplist_strings_util.mm' (0xf8340) of size 7
  '.str117' is ascii string 'PATCH='
SUMMARY: AddressSanitizer: global-buffer-overflow ??:0 wrap_memmove
Shadow bytes around the buggy address:
  0x2001f010: 03 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 02 f9 f9 f9
  0x2001f020: 03 f9 f9 f9 02 f9 f9 f9 03 f9 f9 f9 02 f9 f9 f9
  0x2001f030: 03 f9 f9 f9 02 f9 f9 f9 03 f9 f9 f9 f9 f9 f9 f9
  0x2001f040: 00 00 00 00 00 05 f9 f9 f9 f9 f9 f9 00 00 00 00
  0x2001f050: 00 00 00 00 00 00 00 00 07 f9 f9 f9 f9 f9 f9 f9
=>0x2001f060: 07 f9 f9 f9 07 f9 f9 f9 07 f9 f9 f9[f9]04 f9 f9
  0x2001f070: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00
  0x2001f080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x2001f090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x2001f0a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x2001f0b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Contiguous container OOB:fc
  ASan internal:           fe
==38424==ABORTING
ninja: build stopped: subcommand failed.

Original issue reported on code.google.com by ramosian.glider@gmail.com on 13 Mar 2014 at 9:58

[GoogleCodeExporter]

From a local run with ASAN_OPTIONS=report_globals=2:

...
==55507==Added Global: beg=0x00156340 size=7/64 name=.str117 
module=../../chrome/tools/mac_helpers/infoplist_strings_util.mm dyn_init=0
==55507==Added Global: beg=0x00156360 size=12/64 name=.str119 
module=../../chrome/tools/mac_helpers/infoplist_strings_util.mm dyn_init=0
...
==55507==Search Global: beg=0x00156360 size=12/64 name=.str119 
module=../../chrome/tools/mac_helpers/infoplist_strings_util.mm dyn_init=0
0x00156362 is located 2 bytes inside of global variable '.str119' from 
'../../chrome/tools/mac_helpers/infoplist_strings_util.mm' (0x156360) of size 12
  '.str119' is ascii string '%d.%d.%d.%d'
==55507==Search Global: beg=0x00156340 size=7/64 name=.str117 
module=../../chrome/tools/mac_helpers/infoplist_strings_util.mm dyn_init=0
0x00156362 is located 27 bytes to the right of global variable '.str117' from 
'../../chrome/tools/mac_helpers/infoplist_strings_util.mm' (0x156340) of size 7
  '.str117' is ascii string 'PATCH='
...

According to -emit-llvm the size of .str117 is 64 bytes:

@.str117 = internal unnamed_addr constant { [7 x i8], [57 x i8] } { [7 x i8] 
c"PATCH=\00", [57 x i8] zeroinitializer }, section 
"__TEXT,__cstring,cstring_literals", align 32

, however the next string starts at .str117+32.

Hexdump also shows that the variables are too close to each other:

00079340  50 41 54 43 48 3d 00 00  00 00 00 00 00 00 00 00  |PATCH=..........|
00079350  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
00079360  25 64 2e 25 64 2e 25 64  2e 25 64 00 00 00 00 00  |%d.%d.%d.%d.....|

Original comment by ramosian.glider@gmail.com on 13 Mar 2014 at 10:03

[GoogleCodeExporter]

The assembly looks sane:

        .section        __TEXT,__cstring,cstring_literals
        .align  5                       ## @.str117
_.str117:
        .asciz  "PATCH="
        .space  57

        .section        __DATA,__cfstring
        .align  3                       ## @_unnamed_cfstring_118
L__unnamed_cfstring_118:
        .long   ___CFConstantStringClassReference
        .long   1992                    ## 0x7c8
        .long   _.str117
        .long   6                       ## 0x6

        .section        __TEXT,__cstring,cstring_literals
        .align  5                       ## @.str119
_.str119:
        .asciz  "%d.%d.%d.%d"
        .space  52

Perhaps the linker removes the padding, but keeps the alignment?

Original comment by ramosian.glider@gmail.com on 13 Mar 2014 at 10:14

[GoogleCodeExporter]

This is a regression caused by the recent changes to string handling.

According to https://code.google.com/p/address-sanitizer/issues/detail?id=32 
the strings in __TEXT,__cstring,cstring_literals were previously marked as 
linker_private and thus not instrumented. Now they've become internal, so ASan 
instruments them.

However strings in the cstring_literals section are mergeable:

"""
A cstring_literals section contains null-terminated literal C language 
character strings. The link editor places only one copy of each literal into 
the output file’s section and relocates references to different copies of the 
same literal to the one copyin the output file. There can be no relocation 
entries for a section of this type, and all references to literals in this 
section must be inside the address range for the specific literal being 
referenced. The last byte in a section of this type must be a null byte, and 
the strings can’t contain null bytes in their bodies. An example of a 
cstring_literals section is one for the literal strings that appear in the body 
of an ANSI C function where the compiler chooses to make such strings read only.
"""

(from the Mac OS Assembler Guide)

, thus we should not instrument them.

Original comment by ramosian.glider@gmail.com on 13 Mar 2014 at 12:06

[GoogleCodeExporter]

Minimal repro below:

#import <Foundation/Foundation.h>

#include <stdio.h>

int main() {
  NSString* version_file = @"MAJOR=35\n";
  int major = 0, minor = 0, build = 0, patch = 0;
  NSScanner* scanner = [NSScanner scannerWithString:version_file];
  NSString *res = nil;
  if ([scanner scanString:@"MAJOR=" intoString:nil] &&
      [scanner scanInt:&major]) {
    res = [NSString stringWithFormat:@"%d.%d.%d.%d",
           major, minor, build, patch];
  }
  printf("%s\n", [res UTF8String]);
  return 0;
}

Original comment by ramosian.glider@gmail.com on 13 Mar 2014 at 12:06

[GoogleCodeExporter]

Original comment by gli...@chromium.org on 13 Mar 2014 at 12:51

[GoogleCodeExporter]

Fixed in Clang r203916.

Original comment by ramosian.glider@gmail.com on 14 Mar 2014 at 1:51

• Changed state: Fixed