Skip to content

Commit a9bc692

Browse files
committed
opal/patcher: fix xlc support
The xlc compiler seems to behave in a different way that gcc when it comes the inline asm. There were two problems with the code with xlc: - The TOC read in mca_patcher_base_patch_hook used the syntax register unsigned long toc asm("r2") to read $r2 (the TOC pointer). With gcc this seems to behave as expected but with xlc the result in toc is not the same as $r2. I updated the code to use asm volatile ("std 2, %0" : "=m" (toc)) to load the TOC pointer. - The OPAL_PATCHER_BEGIN macro is meant to be the first thing in a hook. On PPC64 it loads the correct TOC pointer (thanks to mca_patcher_base_patch_hook) and saves the old one. The OPAL_PATCHER_END macro restores the TOC pointer. Because we *need* the TOC to be correct before it is accessed in the hook the OPAL_PATCHER_BEGIN macro MUST come first. We did this and all was well with gcc. With xlc on the other hand there was a TOC access before the assembly inserted by OPAL_PATCHER_BEGIN. To fix this quickly I broke each hook into a pair of function with the OPAL_PATCHER_* macros on the top level functions. This works around the issue but is not a clean way to fix this. In the future we should 1) either update overwrite to not need this, or 2) figure out why xlc is not inserting the asm before the first TOC read. This fixes #1854 Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
1 parent c082068 commit a9bc692

File tree

2 files changed

+60
-12
lines changed

2 files changed

+60
-12
lines changed

opal/mca/memory/patcher/memory_patcher_component.c

Lines changed: 56 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,12 @@ opal_memory_patcher_component_t mca_memory_patcher_component = {
9898
#define memory_patcher_syscall syscall
9999
#endif
100100

101+
/* All the hooks in this file have two levels. The first level has the OPAL_PATCHER_* macros
102+
* around the call to the second level. This was done because with xlc the compiler was
103+
* generating an access to r2 before the OPAL_PATCHER_* assembly. This was loading invalid
104+
* data. If this can be resolved the two levels can be joined.
105+
*/
106+
101107
/*
102108
* The following block of code is #if 0'ed out because we do not need
103109
* to intercept mmap() any more (mmap() only deals with memory
@@ -148,9 +154,8 @@ static void *intercept_mmap(void *start, size_t length, int prot, int flags, int
148154

149155
static int (*original_munmap) (void *, size_t);
150156

151-
static int intercept_munmap(void *start, size_t length)
157+
static int _intercept_munmap(void *start, size_t length)
152158
{
153-
OPAL_PATCHER_BEGIN;
154159
int result = 0;
155160

156161
/* could be in a malloc implementation */
@@ -162,6 +167,13 @@ static int intercept_munmap(void *start, size_t length)
162167
result = original_munmap (start, length);
163168
}
164169

170+
return result;
171+
}
172+
173+
static int intercept_munmap(void *start, size_t length)
174+
{
175+
OPAL_PATCHER_BEGIN;
176+
int result = _intercept_munmap (start, length);
165177
OPAL_PATCHER_END;
166178
return result;
167179
}
@@ -178,12 +190,11 @@ static void *(*original_mremap) (void *, size_t, void *, size_t, int);
178190
#endif
179191

180192
#if defined(__linux__)
181-
static void *intercept_mremap (void *start, size_t oldlen, size_t newlen, int flags, void *new_address)
193+
static void *_intercept_mremap (void *start, size_t oldlen, size_t newlen, int flags, void *new_address)
182194
#else
183-
static void *intercept_mremap (void *start, size_t oldlen, void *new_address, size_t newlen, int flags)
195+
static void *_intercept_mremap (void *start, size_t oldlen, void *new_address, size_t newlen, int flags)
184196
#endif
185197
{
186-
OPAL_PATCHER_BEGIN;
187198
void *result = MAP_FAILED;
188199

189200
if (MAP_FAILED != start && oldlen > 0) {
@@ -210,19 +221,35 @@ static void *intercept_mremap (void *start, size_t oldlen, void *new_address, si
210221
}
211222
#endif
212223

224+
return result;
225+
}
226+
227+
#if defined(__linux__)
228+
static void *intercept_mremap (void *start, size_t oldlen, size_t newlen, int flags, void *new_address)
229+
{
230+
OPAL_PATCHER_BEGIN;
231+
void *result = _intercept_mremap (start, oldlen, newlen, flags, new_address);
213232
OPAL_PATCHER_END;
214233
return result;
215234
}
235+
#else
236+
static void *intercept_mremap (void *start, size_t oldlen, void *new_address, size_t newlen, int flags)
237+
{
238+
OPAL_PATCHER_BEGIN;
239+
void *result = _intercept_mremap (start, oldlen, new_address, newlen, flags);
240+
OPAL_PATCHER_END;
241+
return result;
242+
}
243+
#endif
216244

217245
#endif
218246

219247
#if defined (SYS_madvise)
220248

221249
static int (*original_madvise) (void *, size_t, int);
222250

223-
static int intercept_madvise (void *start, size_t length, int advice)
251+
static int _intercept_madvise (void *start, size_t length, int advice)
224252
{
225-
OPAL_PATCHER_BEGIN;
226253
int result = 0;
227254

228255
if (advice == MADV_DONTNEED ||
@@ -240,6 +267,12 @@ static int intercept_madvise (void *start, size_t length, int advice)
240267
result = original_madvise (start, length, advice);
241268
}
242269

270+
return result;
271+
}
272+
static int intercept_madvise (void *start, size_t length, int advice)
273+
{
274+
OPAL_PATCHER_BEGIN;
275+
int result = _intercept_madvise (start, length, advice);
243276
OPAL_PATCHER_END;
244277
return result;
245278
}
@@ -254,9 +287,8 @@ extern void *__curbrk; /* in libc */
254287

255288
static int (*original_brk) (void *);
256289

257-
static int intercept_brk (void *addr)
290+
static int _intercept_brk (void *addr)
258291
{
259-
OPAL_PATCHER_BEGIN;
260292
int result = 0;
261293
void *old_addr, *new_addr;
262294

@@ -293,6 +325,13 @@ static int intercept_brk (void *addr)
293325
} else if (new_addr < old_addr) {
294326
opal_mem_hooks_release_hook (new_addr, (intptr_t) old_addr - (intptr_t) new_addr, true);
295327
}
328+
return result;
329+
}
330+
331+
static int intercept_brk (void *addr)
332+
{
333+
OPAL_PATCHER_BEGIN;
334+
int result = _intercept_brk (addr);
296335
OPAL_PATCHER_END;
297336
return result;
298337
}
@@ -364,9 +403,8 @@ static size_t memory_patcher_get_shm_seg_size (const void *shmaddr)
364403

365404
static int (*original_shmdt) (const void *);
366405

367-
static int intercept_shmdt (const void *shmaddr)
406+
static int _intercept_shmdt (const void *shmaddr)
368407
{
369-
OPAL_PATCHER_BEGIN;
370408
int result;
371409

372410
/* opal_mem_hooks_release_hook should probably be updated to take a const void *.
@@ -379,6 +417,13 @@ static int intercept_shmdt (const void *shmaddr)
379417
result = memory_patcher_syscall (SYS_shmdt, shmaddr);
380418
}
381419

420+
return result;
421+
}
422+
423+
static int intercept_shmdt (const void *shmaddr)
424+
{
425+
OPAL_PATCHER_BEGIN;
426+
int result = _intercept_shmdt (shmaddr);
382427
OPAL_PATCHER_END;
383428
return result;
384429
}

opal/mca/patcher/base/patcher_base_patch.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,10 @@ int mca_patcher_base_patch_hook (mca_patcher_base_module_t *module, uintptr_t ho
175175
}
176176

177177
// generate code to restore TOC
178-
register unsigned long toc asm("r2");
178+
unsigned long toc;
179+
180+
asm volatile ("std 2, %0" : "=m" (toc));
181+
179182
hook_patch->patch_data_size = PatchLoadImm((uintptr_t)hook_patch->patch_data, 2, toc);
180183

181184
/* put the hook patch on the patch list so it will be undone on finalize */

0 commit comments

Comments
 (0)