#38 Fix unitialized memory access found by valgrind

closed-accepted
nobody
None
5
2004-12-28
2004-12-11
Miloslav Trmac
No

Hello,
the attached patch fixes some uninitialized memory
accesses:
* pdl_initthreadstruct () checks whether the magic
value is correct,
but the value might be correct by accident.
Calls to PDL_THR_CLRMAGIC after every pdl_thread
allocation
fix this. I don't understand PP much, so there might
be a better way
than defining a new config setting...
* threadI_redodims would access threadids[-1].

This might be related to #1051810, but it's just a code
cleanup,
not fixing the deep causes.

Discussion

  • Miloslav Trmac
    Miloslav Trmac
    2004-12-11

    Patch against PDL-2.4.1

     
    Attachments
  • Craig DeForest
    Craig DeForest
    2004-12-14

    Logged In: YES
    user_id=20200

    Hmmm... What, exactly, is the NoPdlThread option doing? It
    looks like it's preventing PDL_THR_CLRMAGIC from getting
    called -- but it's not obvious (yet) why that should be so.
    Could you send a paragraph or so explaining why the
    slices.pd calls should have NoPdlThread set to 1 (and other
    sorts of PP definitions should have NoPdlThread set to 0)?

    Cheers,
    Craig

     
  • Miloslav Trmac
    Miloslav Trmac
    2004-12-14

    Logged In: YES
    user_id=603514

    The slices.pd operations do not have the __pdlthread member, so
    unconditional calling of PDL_THR_CLRMAGIC results in compile
    errors.

    There should be a way to recognize this without the NoPdlThread
    option (like the existence of __pdlthread is determined without
    the option), but I had a hard time just getting this option
    working.

     
  • Craig DeForest
    Craig DeForest
    2004-12-14

    Logged In: YES
    user_id=20200

    Gotcha. I have to admit that the internals of PP are a bit
    impenetrable to me -- I probably need to take a week and
    just grovel around through it to figure it all out.

    The NoPDLThread option seems like a straightforward way to
    proceed for now, and I've checked it into CVS. The main
    thing remaining is to document its use so folks know what
    the heck it's for. Would you like to stick a similar note
    into the the nice (if somewhat incomplete) PP write-up in
    the PP web page? I can probably gin up something, but as
    you were just in the code you probably have a better sense
    of what it's doing.

    I notice that all of the slices.pd routines have the
    P2Child=>1 field set, and none of the others in the
    distribution have that option. Is that the switch you're
    looking for?

    Cheers,
    Craig

     
  • Miloslav Trmac
    Miloslav Trmac
    2004-12-19

    Logged In: YES
    user_id=603514

    I'm not sure about P2Child; basically I have no idea how
    HaveThreading, set by NewParentChildPars, depends on P2Child.

    I'm not quite sure whether the attached PP.pod patch is
    what you were asking for, but it's basically all I know
    about the flag.

     
  • Miloslav Trmac
    Miloslav Trmac
    2004-12-19

    A stub documentation for NoPdlThread

     
    Attachments
  • Craig DeForest
    Craig DeForest
    2004-12-28

    Logged In: YES
    user_id=20200

    Sorry not to have closed this out before -- it did make it
    into 2.4.2, thanks very much!

     
  • Craig DeForest
    Craig DeForest
    2004-12-28

    • status: open --> closed-accepted