Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

README.valgrind should mention --with-valgrind #63059

Closed
tim-one opened this issue Aug 28, 2013 · 9 comments
Closed

README.valgrind should mention --with-valgrind #63059

tim-one opened this issue Aug 28, 2013 · 9 comments
Assignees
Labels
3.8 only security fixes build The build process and cross-build easy type-feature A feature request or enhancement

Comments

@tim-one
Copy link
Member

tim-one commented Aug 28, 2013

BPO 18859
Nosy @tim-one, @ambv, @elenaoat, @CuriousLearner
PRs
  • bpo-18859: Document --with-valgrind option in README.valgrind #10591
  • Files
  • patch.diff: Patch
  • issue18859.diff
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/ambv'
    closed_at = <Date 2018-11-20.11:12:05.530>
    created_at = <Date 2013-08-28.00:19:48.855>
    labels = ['easy', 'type-feature', '3.8', 'build']
    title = 'README.valgrind should mention --with-valgrind'
    updated_at = <Date 2018-11-20.11:12:05.518>
    user = 'https://github.com/tim-one'

    bugs.python.org fields:

    activity = <Date 2018-11-20.11:12:05.518>
    actor = 'lukasz.langa'
    assignee = 'lukasz.langa'
    closed = True
    closed_date = <Date 2018-11-20.11:12:05.530>
    closer = 'lukasz.langa'
    components = ['Build']
    creation = <Date 2013-08-28.00:19:48.855>
    creator = 'tim.peters'
    dependencies = []
    files = ['36209', '42147']
    hgrepos = []
    issue_num = 18859
    keywords = ['patch', 'easy']
    message_count = 9.0
    messages = ['196338', '196548', '224559', '224573', '261647', '294386', '330109', '330125', '330126']
    nosy_count = 7.0
    nosy_names = ['tim.peters', 'mmokrejs', 'lukasz.langa', 'Elena.Oat', 'sowmya-ravidas', 'CuriousLearner', 'Sourav Singh']
    pr_nums = ['10591']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue18859'
    versions = ['Python 3.8']

    @tim-one
    Copy link
    Member Author

    tim-one commented Aug 28, 2013

    In bpo-18843 a user noted that Misc/README.valgrind doesn't mention the --with-valgrind configure option. It probably should. But since I've never used valgrind, I'm not the guy to do it ;-)

    @tim-one tim-one added build The build process and cross-build easy labels Aug 28, 2013
    @mmokrejs
    Copy link
    Mannequin

    mmokrejs mannequin commented Aug 30, 2013

    Moreover, it should explain what that really does. One could think of several answers or even their combinations what this configure flag will really do:

    a) python will run itself under valgrind, don't bother ever doing it yourself
    b) you don't have to bother with uncommenting the lines in valgrind.supp
    c) "make install" will install the valgrind.supp but you have to edit it on your own
    d) you may use valgrind but don't use other tool replacing malloc(), like electric fence, DUMA, etc. You should be quite verbose how (in)compatible is this with other tools.

    Or some combination of them?

    valgrind docs say it won't work if a binary lacks debug symbols (wasn't compiled with -g). Initially I got mislead I though I have to convert my blah.py to blah.c using cython, compile that with "gcc -ggdb blah.c" and then I may run valgrind on the binary.

    In the end, I don't understand why everybody has to remove the comment symbols from the valgrind-python.supp file at all. Why isn't that by default enabled?

    I also suggest you mention right in the file other handy information because people quite likely get on this path while chasing memory corruption issues, maybe broken hardware:

    1. Mention that python uses 256kB chunks by default.
    2. Mention there exists --without-pymalloc as Tim explained me in http://bugs.python.org/issue18843#msg196492
    3. Explain how to interpret those stacktraces one could get: http://bugs.python.org/issue18843#msg196481

    Thank you!

    @ezio-melotti ezio-melotti added the type-feature A feature request or enhancement label Sep 4, 2013
    @sowmya-ravidas
    Copy link
    Mannequin

    sowmya-ravidas mannequin commented Aug 2, 2014

    Hi,

    I have created a patch for this bug.
    The Misc/README.valgrind now mentions the --with-valgrind configure options.

    @sowmya-ravidas sowmya-ravidas mannequin removed the type-feature A feature request or enhancement label Aug 2, 2014
    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Aug 2, 2014

    Hi Sowmya. Currently we have the option to use --with-valgrind or
    the old method --without-pymalloc. Both methods work.

    @berkerpeksag berkerpeksag added the type-feature A feature request or enhancement label Oct 14, 2014
    @elenaoat
    Copy link
    Mannequin

    elenaoat mannequin commented Mar 12, 2016

    I've combined the patch submitted by Sowmya and also added there clarifications from points 1, 2 mentioned by Martin Mokrejs.

    @ambv ambv self-assigned this Feb 3, 2017
    @SouravSingh
    Copy link
    Mannequin

    SouravSingh mannequin commented May 24, 2017

    Is the issue fixed?

    @CuriousLearner
    Copy link
    Member

    Patch added in PR: #10591

    @ambv
    Copy link
    Contributor

    ambv commented Nov 20, 2018

    New changeset d5d3368 by Łukasz Langa (Sanyam Khurana) in branch 'master':
    bpo-18859: Document --with-valgrind option in README.valgrind (bpo-10591)
    d5d3368

    @ambv
    Copy link
    Contributor

    ambv commented Nov 20, 2018

    Thanks! ✨ 🍰 ✨

    @ambv ambv added the 3.8 only security fixes label Nov 20, 2018
    @ambv ambv closed this as completed Nov 20, 2018
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 only security fixes build The build process and cross-build easy type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants