Skip to content

Fix nrt debug print - #7027

Merged
sklam merged 2 commits into
numba:masterfrom
MegaIng:fix_nrt_debug_print
May 19, 2021
Merged

sklam merged 2 commits into
numba:masterfrom
MegaIng:fix_nrt_debug_print

Conversation

@MegaIng

@MegaIng MegaIng commented May 14, 2021

Copy link
Copy Markdown
Contributor

This PR is a byproduct of me trying to fix generators. Without this PR I get output like this

*** NRT_Decref 140720103881220 [000002C1CB230770]
*** NRT_Incref 29778180 [000002C1CB230770] 
*** NRT_Decref 140720103881221 [000002C1CB230770] 

This is because the call to printf was essentially

printf("*** NRT_Decref %zu [%p]\n", *ptr, ptr)

The problem here is that the type of ptr is int8*., meaning the a value of type int8 was passed to printf when it expected a value of type uint64. This is undefined behaviour and apparently didn't break for others. I now changed it to be:

printf("*** NRT_Decref %zu [%p]\n", *((uint64*) ptr), ptr)

which does work correctly for me.

At request of @gmarkall I also added a config variable 'NUMBA_DEBUG_NRT' to replace the global _debug_print in nrtdynmod

@sklam

sklam commented May 14, 2021

Copy link
Copy Markdown
Member

Good catch on the %zu mismatch. Thanks!

@sklam sklam added this to the Numba 0.54 RC milestone May 14, 2021

@stuartarchibald stuartarchibald left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch!

@stuartarchibald stuartarchibald added 5 - Ready to merge Review and testing done, is ready to merge and removed 3 - Ready for Review labels May 19, 2021

@sklam sklam left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@sklam
sklam merged commit e314821 into numba:master May 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

5 - Ready to merge Review and testing done, is ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants