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

Error disappears when a module is added #31

Open
bgamari opened this issue Jul 4, 2018 · 5 comments
Open

Error disappears when a module is added #31

bgamari opened this issue Jul 4, 2018 · 5 comments
Labels
Milestone

Comments

@bgamari
Copy link
Collaborator

bgamari commented Jul 4, 2018

Using 05b02cf I am seeing some rather concerning behavior when processing callmaps. I am using this configuration file:

sm_lock_held      "assume the storage manager lock is held";                
take_sm_lock      "the ability to take the storage manager lock"            
  -> ! sm_lock_held;                                                        
                                                                            
.enforce "sm/NonMovingMark.c";                                              
.enforce "sm/NonMovingMark.h";                                              
.enforce "sm/BlockAlloc.c";                                                 
.enforce "rts/sm/BlockAlloc.h";                                             
.enforce "Block.h";                                                         

I have two callmap files: rts/sm/BlockAlloc.c.ward.graph and rts/Capability.c.ward.graph.

Checking the Capability callmap alone correctly reports no errors:

$ ward gcc --mode=compiler --config=rts/config.ward    rts/Capability.c.ward.graph   
Loading config files...
Preprocessing and parsing...
Checking...
Warnings: 0, Errors: 0

Checking the BlockAlloc callmap alone correctly reports several errors:

$ ward gcc --mode=compiler --config=rts/config.ward rts/sm/BlockAlloc.c.ward.graph                                                                                                                                                       
Loading config files...
Preprocessing and parsing...
Checking...
rts/sm/BlockAlloc.h:13: error: missing required annotation on 'allocLargeChunk'; annotation [] is missing: [revoke(take_sm_lock),need(take_sm_lock),grant(sm_lock_held)]
includes/rts/storage/Block.h:320: error: missing required annotation on 'allocGroupOnNode_lock'; annotation [] is missing: [need(take_sm_lock)]
includes/rts/storage/Block.h:315: error: missing required annotation on 'allocGroup_lock'; annotation [] is missing: [need(take_sm_lock)]
includes/rts/storage/Block.h:317: error: missing required annotation on 'allocBlock_lock'; annotation [] is missing: [need(take_sm_lock)]
includes/rts/storage/Block.h:322: error: missing required annotation on 'allocBlockOnNode_lock'; annotation [] is missing: [need(take_sm_lock)]
includes/rts/storage/Block.h:333: error: missing required annotation on 'freeGroup_lock'; annotation [] is missing: [need(take_sm_lock)]
includes/rts/storage/Block.h:335: error: missing required annotation on 'freeChain_lock'; annotation [] is missing: [need(take_sm_lock)]
Warnings: 0, Errors: 7

However, when I check the Capability and BlockAlloc callmaps together the error vanishes:

$ ward gcc --mode=compiler --config=rts/config.ward rts/Capability.c.ward.graph rts/sm/BlockAlloc.c.ward.graph
Loading config files...
Preprocessing and parsing...
Checking...
Warnings: 0, Errors: 0

Even stranger, when I flip the order of the two the errors are again correctly reported,

$ ward gcc --mode=compiler --config=rts/config.ward rts/sm/BlockAlloc.c.ward.graph rts/Capability.c.ward.graph
Loading config files...
Preprocessing and parsing...
Checking...
rts/sm/BlockAlloc.h:13: error: missing required annotation on 'allocLargeChunk'; annotation [] is missing: [revoke(take_sm_lock),need(take_sm_lock),grant(sm_lock_held)]
includes/rts/storage/Block.h:320: error: missing required annotation on 'allocGroupOnNode_lock'; annotation [] is missing: [need(take_sm_lock)]
includes/rts/storage/Block.h:315: error: missing required annotation on 'allocGroup_lock'; annotation [] is missing: [need(take_sm_lock)]
includes/rts/storage/Block.h:317: error: missing required annotation on 'allocBlock_lock'; annotation [] is missing: [need(take_sm_lock)]
includes/rts/storage/Block.h:322: error: missing required annotation on 'allocBlockOnNode_lock'; annotation [] is missing: [need(take_sm_lock)]
includes/rts/storage/Block.h:333: error: missing required annotation on 'freeGroup_lock'; annotation [] is missing: [need(take_sm_lock)]
includes/rts/storage/Block.h:335: error: missing required annotation on 'freeChain_lock'; annotation [] is missing: [need(take_sm_lock)]
Warnings: 0, Errors: 7
@bgamari
Copy link
Collaborator Author

bgamari commented Jul 4, 2018

I've pushed the two necessary callmap files to https://github.com/bgamari/ward-T31-repro

@bgamari
Copy link
Collaborator Author

bgamari commented Jul 4, 2018

While debugging this I have noticed if I export the callgraph of the failing case I find that allocLargeChunk has no calls. This is likely why the errors aren't being reported.

@bgamari
Copy link
Collaborator Author

bgamari commented Jul 4, 2018

Indeed it looks like declarations aren't handled properly. Specifically, Capability.c.ward.graph contains:

(function (ident "allocLargeChunk" (node 40671 (span (source "rts/sm/BlockAlloc.h" 248117 13 9) 15 (source "rts/sm/BlockAlloc.h" 248117 13 9)))) (node 40685 (span (source "rts/sm/BlockAlloc.h" 248109 13 1) 1 (source "rts/sm/BlockAlloc.h" 248149 13 41)))                             
(actions )                                                                                                                                                                                                                                                                                
(calltree                                                                                                                                                                                                                                                                                 
))      

that is, just a declaration for allocLargeChunk, which naturally has no calls.

However, BlockAlloc.c.ward.graph contains the definition of allocLargeChunk and consequently contains calls:

(function (ident "allocLargeChunk" (node 38563 (span (source "rts/sm/BlockAlloc.h" 233950 13 9) 15 (source "rts/sm/BlockAlloc.h" 233950 13 9)))) (node 38577 (span (source "rts/sm/BlockAlloc.h" 233942 13 1) 1 (source "rts/sm/BlockAlloc.h" 233982 13 41)))                             
(actions )                                                                                                                                                                                                                                                                                
(calltree                                                                                                                                                                                                                                                                                 
(call (ident "_acquire_sm_lock" (node 43174 (span (source "rts/sm/BlockAlloc.c" 256846 650 2) 16 (source "rts/sm/BlockAlloc.c" 256846 650 2)))))                                                                                                                                          
(call (ident "rts/sm/BlockAlloc.c`nodeWithLeastBlocks" (node 43180 (span (source "rts/sm/BlockAlloc.c" 256899 651 34) 19 (source "rts/sm/BlockAlloc.c" 256899 651 34)))))                                                                                                                 
(call (ident "allocLargeChunkOnNode" (node 43178 (span (source "rts/sm/BlockAlloc.c" 256877 651 12) 21 (source "rts/sm/BlockAlloc.c" 256877 651 12)))))))                                                                                                                                 
(function (ident "freeWSDeque" (node 36212 (span (source "rts/WSDeque.h" 220424 70 6) 11 (source "rts/WSDeque.h" 220424 70 6)))) (node 36222 (span (source "rts/WSDeque.h" 220419 70 1) 1 (source "rts/WSDeque.h" 220448 70 30)))                                                         
(actions )                                                                                                                                                                                                                                                                                
(calltree                                                                                                                                                                                                                                                                                 
))                                                                                                                                                                                                                                                                                        

It looks like depending upon the order the files are given in the declaration may override the definition.

@lambdageek
Copy link
Collaborator

I don't have a working Ward at the moment, but the first thing to check is whether this works out with simple examples smaller than the GHC rts.

The second thing to check is whether this is related to GHCs reliance on #pragma once instead of include guards and whether that's somehow messes up the graph merging. We've only ever tests on code that uses traditional include guards.

bgamari added a commit to bgamari/Ward that referenced this issue Jul 6, 2018
Previously the calls (or, rather, lack thereof) of a function's declaration would
override those of its definition. This resulted in evincarofautumn#31.
@bgamari
Copy link
Collaborator Author

bgamari commented Jul 6, 2018

To be clear, I described the underlying issue in my previous comment. It seems that the treatment of declarations and definitions is simply wrong.

I just pushed #35 which fixes the issue.

@evincarofautumn evincarofautumn added this to the 1.0 milestone Oct 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants