|
From: Florian K. <fl...@ei...> - 2015-04-18 18:30:24
|
I noticed that the do_clone function in syswrap-tilegx-linux.c
does its own stack guessing and registration instead of using
the generic ML_(guess_and_register_stack) function which is
used by all other archs for that purpose. If at all possible
we should strive to use the generic code unless there is a *really*
good reason not to..
Looking at your code I see some subtle differences and am wondering
whether those are correct:
tilegx code:
ctst->client_stack_highest_byte = (Addr) VG_PGROUNDUP (sp);
ctst->client_stack_szB = ctst->client_stack_highest_byte - seg->start;
generic code:
tst->client_stack_highest_byte = (Addr)VG_PGROUNDUP(sp)-1;
tst->client_stack_szB = tst->client_stack_highest_byte - seg->start + 1;
That looks like an off-by-one error to me. Could you elaborate?
I'm thinking about the patch below.
Florian
Index: coregrind/m_syswrap/syswrap-tilegx-linux.c
===================================================================
--- coregrind/m_syswrap/syswrap-tilegx-linux.c (revision 15111)
+++ coregrind/m_syswrap/syswrap-tilegx-linux.c (working copy)
@@ -337,7 +337,6 @@
ThreadState * ctst = VG_ (get_ThreadState) (ctid);
Long ret = 0;
Long * stack;
- NSegment const *seg;
SysRes res;
vki_sigset_t blockall, savedmask;
@@ -372,23 +371,8 @@
See #226116. */
ctst->os_state.threadgroup = ptst->os_state.threadgroup;
- seg = VG_ (am_find_nsegment) ((Addr) sp);
+ ML_(guess_and_register_stack) (sp, ctst);
- if (seg && seg->kind != SkResvn) {
- ctst->client_stack_highest_byte = (Addr) VG_PGROUNDUP (sp);
- ctst->client_stack_szB = ctst->client_stack_highest_byte - seg->start;
- VG_ (register_stack) (seg->start, ctst->client_stack_highest_byte);
- if (debug)
- VG_ (printf) ("tid %d: guessed client stack range %#lx-%#lx\n",
-
- ctid, seg->start, VG_PGROUNDUP (sp));
- } else {
- VG_ (message) (Vg_UserMsg,
- "!? New thread %d starts with sp+%#lx) unmapped\n",
- ctid, sp);
- ctst->client_stack_szB = 0;
- }
-
VG_TRACK (pre_thread_ll_create, ptid, ctid);
if (flags & VKI_CLONE_SETTLS) {
if (debug)
|