Skip to content

Commit a9990f8

Browse files
peffgitster
authored andcommitted
Makefile: turn on NO_MMAP when building with ASan
Git often uses mmap() to access on-disk files. This leaves a blind spot in our SANITIZE=address builds, since ASan does not seem to handle mmap at all. Nor does the OS notice most out-of-bounds access, since it tends to round up to the nearest page size (so depending on how big the map is, you might have to overrun it by up to 4095 bytes to trigger a segfault). The previous commit demonstrates a memory bug that we missed. We could have made a new test where the out-of-bounds access was much larger, or where the mapped file ended closer to a page boundary. But the point of running the test suite with sanitizers is to catch these problems without having to construct specific tests. Let's enable NO_MMAP for our ASan builds by default, which should give us better coverage. This does increase the memory usage of Git, since we're copying from the filesystem into heap. But the repositories in the test suite tend to be small, so the overhead isn't really noticeable (and ASan already has quite a performance penalty). There are a few other known bugs that this patch will help flush out. However, they aren't directly triggered in the test suite (yet). So it's safe to turn this on now without breaking the test suite, which will help us add new tests to demonstrate those other bugs as we fix them. Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 4deb882 commit a9990f8

File tree

2 files changed

+8
-1
lines changed

2 files changed

+8
-1
lines changed

Makefile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1518,6 +1518,7 @@ SANITIZE_LEAK = YesCompiledWithIt
15181518
endif
15191519
ifneq ($(filter address,$(SANITIZERS)),)
15201520
NO_REGEX = NeededForASAN
1521+
NO_MMAP = NeededForASAN
15211522
SANITIZE_ADDRESS = YesCompiledWithIt
15221523
endif
15231524
endif

meson.build

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1369,12 +1369,18 @@ if host_machine.system() == 'windows'
13691369
libgit_c_args += '-DUSE_WIN32_MMAP'
13701370
else
13711371
checkfuncs += {
1372-
'mmap' : ['mmap.c'],
13731372
# provided by compat/mingw.c.
13741373
'unsetenv' : ['unsetenv.c'],
13751374
# provided by compat/mingw.c.
13761375
'getpagesize' : [],
13771376
}
1377+
1378+
if get_option('b_sanitize').contains('address')
1379+
libgit_c_args += '-DNO_MMAP'
1380+
libgit_sources += 'compat/mmap.c'
1381+
else
1382+
checkfuncs += { 'mmap': ['mmap.c'] }
1383+
endif
13781384
endif
13791385

13801386
foreach func, impls : checkfuncs

0 commit comments

Comments
 (0)