Skip to content

Commit 0e382b0

Browse files
ivankylukivankyluk
authored
i#7753: change emulate_app_brk() to return the user requested address instead of the page aligned address. (#7754)
emulate_app_brk() returning page size aligned address may alter application behavior. For example SPEC 2017 gcc benchmark tests reduce the number of SYS_brk syscalls when page size aligned addresses are returned. Update CI test code_api|linux.brk to include non page size aligned increments and decrements. Fix: #7753
1 parent d21502e commit 0e382b0

File tree

3 files changed

+74
-18
lines changed

3 files changed

+74
-18
lines changed

‎core/unix/os.c‎

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -369,10 +369,16 @@ static int fd_add_pre_heap_flags[MAX_FD_ADD_PRE_HEAP];
369369
static int num_fd_add_pre_heap;
370370

371371
#ifdef LINUX
372+
/* XXX: For self-protection where .data is read-only, these variables should be
373+
* moved to a different data segment or we need to unprotect them every time we
374+
* write to them.
375+
*/
372376
/* i#1004: brk emulation */
373377
static byte *app_brk_map;
374378
static byte *app_brk_cur;
379+
// app_brk_end is the page-aligned upper bound of app_brk_cur.
375380
static byte *app_brk_end;
381+
DECLARE_CXTSWPROT_VAR(static mutex_t app_brk_lock, INIT_LOCK_FREE(app_brk_lock));
376382
#endif
377383

378384
#ifdef MACOS
@@ -1490,6 +1496,9 @@ os_slow_exit(void)
14901496

14911497
DELETE_LOCK(set_thread_area_lock);
14921498
DELETE_LOCK(client_tls_lock);
1499+
#ifdef LINUX
1500+
DELETE_LOCK(app_brk_lock);
1501+
#endif
14931502
IF_NO_MEMQUERY(memcache_exit());
14941503
}
14951504

@@ -3361,7 +3370,9 @@ void
33613370
init_emulated_brk(app_pc exe_end)
33623371
{
33633372
ASSERT(DYNAMO_OPTION(emulate_brk));
3373+
d_r_mutex_lock(&app_brk_lock);
33643374
if (app_brk_map != NULL) {
3375+
d_r_mutex_unlock(&app_brk_lock);
33653376
return;
33663377
}
33673378
/* i#1004: emulate brk via a separate mmap. The real brk starts out empty, but
@@ -3380,46 +3391,51 @@ init_emulated_brk(app_pc exe_end)
33803391
app_brk_end = app_brk_map + BRK_INITIAL_SIZE;
33813392
LOG(GLOBAL, LOG_HEAP, 1, "%s: initial brk is " PFX "-" PFX "\n", __FUNCTION__,
33823393
app_brk_cur, app_brk_end);
3394+
d_r_mutex_unlock(&app_brk_lock);
33833395
}
33843396

33853397
static byte *
33863398
emulate_app_brk(dcontext_t *dcontext, byte *new_val)
33873399
{
3388-
byte *old_brk = app_brk_cur;
3400+
d_r_mutex_lock(&app_brk_lock);
3401+
byte *old_brk = app_brk_end;
33893402
ASSERT(DYNAMO_OPTION(emulate_brk));
33903403
LOG(THREAD, LOG_HEAP, 2, "%s: cur=" PFX ", requested=" PFX "\n", __FUNCTION__,
33913404
app_brk_cur, new_val);
3392-
new_val = (byte *)ALIGN_FORWARD(new_val, PAGE_SIZE);
3405+
byte *new_val_aligned = (byte *)ALIGN_FORWARD(new_val, PAGE_SIZE);
33933406
if (new_val == NULL || new_val == app_brk_cur ||
33943407
/* Not allowed to shrink below original base */
33953408
new_val < app_brk_map) {
33963409
/* Just return cur val */
3397-
} else if (new_val < app_brk_cur) {
3410+
} else if (new_val_aligned < app_brk_end) {
33983411
/* Shrink */
3399-
if (munmap_syscall(new_val, app_brk_end - new_val) == 0) {
3412+
if (munmap_syscall(new_val_aligned, app_brk_end - new_val_aligned) == 0) {
34003413
app_brk_cur = new_val;
3401-
app_brk_end = new_val;
3414+
app_brk_end = new_val_aligned;
34023415
}
34033416
} else if (new_val < app_brk_end) {
34043417
/* We've already allocated the space */
34053418
app_brk_cur = new_val;
34063419
} else {
34073420
/* Expand */
3408-
byte *remap = (byte *)dynamorio_syscall(SYS_mremap, 4, app_brk_map,
3409-
app_brk_end - app_brk_map,
3410-
new_val - app_brk_map, 0 /*do not move*/);
3421+
byte *remap = (byte *)dynamorio_syscall(
3422+
SYS_mremap, 4, app_brk_map, app_brk_end - app_brk_map,
3423+
new_val_aligned - app_brk_map, 0 /*do not move*/);
34113424
if (mmap_syscall_succeeded(remap)) {
34123425
ASSERT(remap == app_brk_map);
34133426
app_brk_cur = new_val;
3414-
app_brk_end = new_val;
3427+
app_brk_end = new_val_aligned;
34153428
} else {
34163429
LOG(THREAD, LOG_HEAP, 1, "%s: mremap to " PFX " failed\n", __FUNCTION__,
34173430
new_val);
34183431
}
34193432
}
3420-
if (app_brk_cur != old_brk)
3421-
handle_app_brk(dcontext, app_brk_map, old_brk, app_brk_cur);
3422-
return app_brk_cur;
3433+
if (app_brk_end != old_brk)
3434+
handle_app_brk(dcontext, app_brk_map, old_brk, app_brk_end);
3435+
3436+
byte *current_brk = app_brk_cur;
3437+
d_r_mutex_unlock(&app_brk_lock);
3438+
return current_brk;
34233439
}
34243440
#endif /* LINUX */
34253441

‎core/utils.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,9 @@ enum {
418418

419419
LOCK_RANK(change_linking_lock), /* < shared_vm_areas, < all heap locks */
420420

421+
# ifdef LINUX
422+
LOCK_RANK(app_brk_lock), /* < shared_vm_areas */
423+
# endif
421424
LOCK_RANK(shared_vm_areas), /* > change_linking_lock, < executable_areas */
422425
LOCK_RANK(shared_cache_count_lock),
423426

‎suite/tests/linux/brk.cpp‎

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,50 @@
4242
* BRK_INITIAL_SIZE is the initial heap size used by init_emulated_brk() (core/unix/os.c).
4343
*/
4444
#define BRK_INITIAL_SIZE 4 * 1024 * 1024
45+
#define BRK_INCREMENT 0x10000
46+
#define BRK_NON_PAGE_ALIGNED_INCREMENT 0x1250
4547

46-
const intptr_t program_break_increments[] = {
47-
0x10000, 0x10000, 0x10000, -0x10000, -0x10000, -0x10000, BRK_INITIAL_SIZE / 2,
48-
0x10000, 0x10000, 0x10000, -0x10000, -0x10000, -0x10000, BRK_INITIAL_SIZE / 2,
49-
0x10000, 0x10000, 0x10000, -0x10000, -0x10000, -0x10000, 0x10000,
50-
0x10000, 0x10000
51-
};
48+
const intptr_t program_break_increments[] = { BRK_INCREMENT,
49+
BRK_INCREMENT,
50+
BRK_INCREMENT,
51+
-BRK_INCREMENT,
52+
-BRK_INCREMENT,
53+
-BRK_INCREMENT,
54+
BRK_NON_PAGE_ALIGNED_INCREMENT,
55+
BRK_NON_PAGE_ALIGNED_INCREMENT,
56+
BRK_NON_PAGE_ALIGNED_INCREMENT,
57+
-BRK_NON_PAGE_ALIGNED_INCREMENT,
58+
-BRK_NON_PAGE_ALIGNED_INCREMENT,
59+
-BRK_NON_PAGE_ALIGNED_INCREMENT,
60+
BRK_INITIAL_SIZE / 2,
61+
BRK_INCREMENT,
62+
BRK_INCREMENT,
63+
BRK_INCREMENT,
64+
-BRK_INCREMENT,
65+
-BRK_INCREMENT,
66+
-BRK_INCREMENT,
67+
BRK_NON_PAGE_ALIGNED_INCREMENT,
68+
BRK_NON_PAGE_ALIGNED_INCREMENT,
69+
BRK_NON_PAGE_ALIGNED_INCREMENT,
70+
-BRK_NON_PAGE_ALIGNED_INCREMENT,
71+
-BRK_NON_PAGE_ALIGNED_INCREMENT,
72+
-BRK_NON_PAGE_ALIGNED_INCREMENT,
73+
BRK_INITIAL_SIZE / 2,
74+
BRK_INCREMENT,
75+
BRK_INCREMENT,
76+
BRK_INCREMENT,
77+
-BRK_INCREMENT,
78+
-BRK_INCREMENT,
79+
-BRK_INCREMENT,
80+
BRK_NON_PAGE_ALIGNED_INCREMENT,
81+
BRK_NON_PAGE_ALIGNED_INCREMENT,
82+
BRK_NON_PAGE_ALIGNED_INCREMENT,
83+
-BRK_NON_PAGE_ALIGNED_INCREMENT,
84+
-BRK_NON_PAGE_ALIGNED_INCREMENT,
85+
-BRK_NON_PAGE_ALIGNED_INCREMENT,
86+
BRK_INCREMENT,
87+
BRK_INCREMENT,
88+
BRK_INCREMENT };
5289

5390
int
5491
main(int argc, const char *argv[])

0 commit comments

Comments
 (0)