|
From: Philippe W. <phi...@sk...> - 2015-04-19 12:29:25
|
On Sat, 2015-04-18 at 20:30 +0200, Florian Krohm wrote:
> 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..
As I understood, the tilegx arch development was done
in the era of valgrind 3.7 or so. At that time
the semantic of stack boundary (bound included or not)
was not consistent/documented.
The semantic was documented and made systematic in r14392 in 2014.
I guess this change was not tracked in the tilegx.
The common/consistent/documented semantic should also be used
for tilegx.
So, the patch below looks a good idea.
Philippe
>
> 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)
>
> ------------------------------------------------------------------------------
> BPM Camp - Free Virtual Workshop May 6th at 10am PDT/1PM EDT
> Develop your own process in accordance with the BPMN 2 standard
> Learn Process modeling best practices with Bonita BPM through live exercises
> http://www.bonitasoft.com/be-part-of-it/events/bpm-camp-virtual- event?utm_
> source=Sourceforge_BPM_Camp_5_6_15&utm_medium=email&utm_campaign=VA_SF
> _______________________________________________
> Valgrind-developers mailing list
> Val...@li...
> https://lists.sourceforge.net/lists/listinfo/valgrind-developers
|