racket / typed-racket

Typed Racket
Other
527 stars 102 forks source link

Bad access in prefab/c #902

Closed LiberalArtist closed 4 years ago

LiberalArtist commented 4 years ago

I have a program that can provoke an internal error (struct-ref: bad access) that appears to be coming from the implementation of prefab/c. Changing some modules from #lang typed/racket to #lang typed/racket/no-check eliminates the error.

Unfortunately, I haven't been able to reproduce this except with a multi-file program that also needs some input data. I'm happy to either try to debug this, if someone can give some suggestions, or to provide enough code and data to reproduce the issue.

Here's the stack trace from 7.5 CS:

struct-ref: bad access
  context...:
   condition->exn
   do-raise
   dynamic-wind
   /Users/pltbuild/build/plt-release-cs-64/bundle/racket/share/pkgs/typed-racket-lib/typed-racket/utils/prefab-c.rkt:53:12: for-loop
   /Users/pltbuild/build/plt-release-cs-64/bundle/racket/collects/racket/contract/private/orc.rkt:274:11: loop
   /Users/pltbuild/build/plt-release-cs-64/bundle/racket/collects/racket/contract/private/orc.rkt:301:9: loop
   /Users/pltbuild/build/plt-release-cs-64/bundle/racket/collects/racket/contract/private/list.rkt:232:16: for-loop
   /Users/pltbuild/build/plt-release-cs-64/bundle/racket/collects/racket/contract/private/list.rkt:230:10
   /Users/pltbuild/build/plt-release-cs-64/bundle/racket/share/pkgs/typed-racket-lib/typed-racket/utils/prefab-c.rkt:86:4: for-loop
   /Users/pltbuild/build/plt-release-cs-64/bundle/racket/share/pkgs/typed-racket-lib/typed-racket/utils/prefab-c.rkt:117:4
   /Users/pltbuild/build/plt-release-cs-64/bundle/racket/collects/racket/contract/combinator.rkt:201:4
   /Users/pltbuild/build/plt-release-cs-64/bundle/racket/collects/racket/contract/private/vector.rkt:255:14: for-loop
   /Users/pltbuild/build/plt-release-cs-64/bundle/racket/collects/racket/contract/private/vector.rkt:232:8: late-neg-proj
   /Users/pltbuild/build/plt-release-cs-64/bundle/racket/share/pkgs/typed-racket-lib/typed-racket/utils/prefab-c.rkt:86:4: for-loop
   /Users/pltbuild/build/plt-release-cs-64/bundle/racket/share/pkgs/typed-racket-lib/typed-racket/utils/prefab-c.rkt:117:4
   /Users/pltbuild/build/plt-release-cs-64/bundle/racket/collects/racket/contract/combinator.rkt:201:4
samth commented 4 years ago

If you can post the code and data somewhere that would be great.

LiberalArtist commented 4 years ago

Ok, I was getting ready to make this code public anyway, so I've set up a branch (mirrored on GitHub as https://github.com/DigitalRicoeur/ricoeur-tei-utils/commit/70b61b52c78af01f4f7ef23a7c556f6069cc7087) with just enough data to reproduce the issue.

After installing the repository as a package, running reproduce-bug.sh prints the stack trace above to standard error: absent the bug, it would exit successfully with no output. (The script must be run from the directory where it's saved.)

I've managed to pinpoint a single file that seems to be where the problem is triggered: the error happens when it is in #lang typed/racket or #lang typed/racket #:no-optimize, but the program succeeds if you change it to #lang typed/racket/no-check.

Thanks for looking at this!

LiberalArtist commented 4 years ago

I've updated the branch for an unrelated compatibility issue with 7.6, and I thought to test both BC and CS. The message from 7.6 BC is more helpful:

Sapientia:secondary-lit philip$ env RACKET=../../bin/racket ./reproduce-bug.sh
Welcome to Racket v7.6.
surname-contributor-ref: index too large
  index: 3
  maximum allowed index: 2
  structure: '#s((surname-contributor person-contributor 2 contributor 1) editor #f #f western #s(styled-name-part "Susen" ("Susen")) #s(styled-name-part "Simon" ("Simon")))
  context...:
   /Users/philip/Desktop/Racket v7.6 BC/share/pkgs/typed-racket-lib/typed-racket/utils/prefab-c.rkt:86:4: for-loop
   /Users/philip/Desktop/Racket v7.6 BC/share/pkgs/typed-racket-lib/typed-racket/utils/prefab-c.rkt:117:4
   /Users/philip/Desktop/Racket v7.6 BC/collects/racket/contract/combinator.rkt:201:4
   /Users/philip/Desktop/Racket v7.6 BC/collects/racket/contract/private/arrow-higher-order.rkt:383:33
   /Users/philip/Desktop/Racket v7.6 BC/tei-utils/secondary-lit/preprocessor/types.rkt:421:0
   /Users/philip/Desktop/Racket v7.6 BC/tei-utils/secondary-lit/preprocessor/parse/common.rkt:198:2: for-loop
   [repeats 1 more time]
   /Users/philip/Desktop/Racket v7.6 BC/tei-utils/secondary-lit/preprocessor/parse/common.rkt:196:0: find/parse-contribs
   /Users/philip/Desktop/Racket v7.6 BC/tei-utils/secondary-lit/preprocessor/parse/book.rkt:125:0: parse-whole-book-meta
   /Users/philip/Desktop/Racket v7.6 BC/tei-utils/secondary-lit/preprocessor/parse/book.rkt:18:0: parse-book-forest
   /Users/philip/Desktop/Racket v7.6 BC/collects/racket/contract/private/arrow-val-first.rkt:486:18
   [repeats 1 more time]
   /Users/philip/Desktop/Racket v7.6 BC/tei-utils/secondary-lit/preprocessor/read.rkt:45:2: read-input
   /Users/philip/Desktop/Racket v7.6 BC/tei-utils/secondary-lit/preprocessor/read.rkt:103:7: for-loop
   /Users/philip/Desktop/Racket v7.6 BC/tei-utils/secondary-lit/preprocessor/read.rkt:88:2: for-loop
   /Users/philip/Desktop/Racket v7.6 BC/tei-utils/secondary-lit/preprocessor/read.rkt:24:0: read-jstor-metadata-files
   ...

And here's the message from 7.6 CS (different line numbers than 7.5 CS, consistent with 7.6 BC):

Sapientia:secondary-lit philip$ ./reproduce-bug.sh 
Welcome to Racket v7.6 [cs].
struct-ref: bad access
  context...:
   condition->exn
   do-raise
   dynamic-wind
   .../utils/prefab-c.rkt:86:4: for-loop
   .../utils/prefab-c.rkt:117:4
   .../contract/combinator.rkt:201:4
   .../private/arrow-higher-order.rkt:383:33
   .../preprocessor/types.rkt:421:0
   .../parse/common.rkt:198:2: for-loop
   [repeats 1 more time]
   .../parse/common.rkt:196:0: find/parse-contribs
   .../parse/book.rkt:125:0: parse-whole-book-meta
   .../parse/book.rkt:18:0: parse-book-forest
   .../private/arrow-val-first.rkt:486:18
   [repeats 1 more time]
   .../preprocessor/read.rkt:45:2: read-input
samth commented 4 years ago

I just tried to reproduce this and wasn't able to. I checked out commit 70b61b5 of that repository, installed it as a package, and ran ./reproduce-bug.sh. I got an error about TODO/void.

LiberalArtist commented 4 years ago

That's the unrelated issue: you need the current typed-racket-issue-902 branch from https://bitbucket.org/digitalricoeur/tei-utils/branch/typed-racket-issue-902 or https://github.com/DigitalRicoeur/ricoeur-tei-utils/tree/typed-racket-issue-902.

LiberalArtist commented 4 years ago

Oh, oops, I see the mirroring to GitHub didn't work because I wasn't pushing from my usual clone. The right commit is https://github.com/DigitalRicoeur/ricoeur-tei-utils/commit/5f3b963.

LiberalArtist commented 4 years ago

(And I've fixed the GitHub mirror now.)

samth commented 4 years ago

Here's a more useful stacktrace:

surname-contributor-ref: index too large
  index: 3
  maximum allowed index: 2
  structure: '#s((surname-contributor person-contributor 2 contributor 1) editor #f #f western #s(styled-name-part "Susen" ("Susen")) #s(styled-name-part "Simon" ("Simon")))
  errortrace...:
   /tmp/ricoeur-tei-utils/secondary-lit/preprocessor/types.rkt:428:3: (surname-contributor type (and prefix (phrase->styled-name-part prefix)) (and suffix (phrase->styled-name-part suffix)) style (phrase->styled-name-part surname) (and given-names (....)))
   /tmp/ricoeur-tei-utils/secondary-lit/preprocessor/types.rkt:427:2: (parsed-contrib (surname-contributor type (and prefix (phrase->styled-name-part prefix)) (and suffix (phrase->styled-name-part ....)) style (phrase->styled-name-part surname) (and ....)))
   /tmp/ricoeur-tei-utils/secondary-lit/preprocessor/parse/book.rkt:147:14: (find/parse-contribs forest)
   /tmp/ricoeur-tei-utils/secondary-lit/preprocessor/parse/book.rkt:144:2: (make-parsed-whole-book-meta #:main-title title #:subtitles subtitles #:contribs (find/parse-contribs forest) #:publishers (find/parse-book-publishers ....) #:pub-dates ....)
   /tmp/ricoeur-tei-utils/secondary-lit/preprocessor/parse/parse.rkt:21:5: (parse-book-forest forest)
   /tmp/ricoeur-tei-utils/secondary-lit/preprocessor/read.rkt:58:10: (parse-book/article-xexpr (read-xexpr/standardized in))
   /tmp/ricoeur-tei-utils/secondary-lit/preprocessor/read.rkt:48:4: (cond ((hash-has-key? checksums-done checksum) (when show-progress? (eprintf "    duplicate checksum; skipping\n"))) (else (hash-set! checksums-done checksum #t) (when show-progress? (eprintf ....)) (define item (....)) (when show-progress? ....) (defin...
   /home/samth/sw/plt/racket/collects/racket/private/kw.rkt:1349:57: (let-values (((pth49) pth) ((read-input50) read-input)) (if (variable-reference-constant? (#%variable-reference call-with-input-file*47)) (call-with-input-file* (quote binary) pth49 read-input50) (call-with-input-file*47 pth49 read-input50)))
   /tmp/ricoeur-tei-utils/secondary-lit/preprocessor/read.rkt:103:7: (for ((pth (in-directory pth)) #:when (xml-path? pth)) (when show-progress? (eprintf "  reading file ~v\n" pth)) (call-with-input-file* pth read-input))
   /tmp/ricoeur-tei-utils/secondary-lit/preprocessor/read.rkt:98:4: (match mode ((quote xml) (call-with-input-file* pth read-input)) ((quote directory) (for ((pth ....) #:when ....) (when ....) (....))) ((quote zip) (call-with-input-file* pth (....))))
   /tmp/ricoeur-tei-utils/secondary-lit/preprocessor/read.rkt:88:2: (for ((pth (in-list args-lst)) (mode (in-list modes-lst)) (i (in-naturals 1))) (define description-of-pth (match mode ((....) ....) (....) ....)) (when show-progress? (eprintf "reading ~a\n" description-of-pth)) (match mode ((quote ....) (....)) ((....)...
   /home/samth/sw/plt/racket/collects/racket/private/kw.rkt:1026:10: (let-values (((call-with-newline-error-handler) (lambda (thunk) (let-values (....) ....)))) (let-values (((store) (make-metadata-parsing-store model))) (let-values (((....) ....)) (let-values ....))))
   /tmp/ricoeur-tei-utils/secondary-lit/preprocessor/write.rkt:24:4: (read-jstor-metadata-files args #:model model #:show-progress? show-progress?)
   /tmp/ricoeur-tei-utils/secondary-lit/preprocessor/interactive.rkt:49:5: (build/write-metadata-archive args #:model model #:output (or output-pth (current-output-port)) #:exists exists-flag #:show-progress? show-progress?)
  context...:
samth commented 4 years ago

Ok, here's a reproduction that doesn't involve Typed Racket or any of your code:

[samth@huor:/tmp/ricoeur-tei-utils/secondary-lit (typed-racket-issue-902) plt] r
Welcome to Racket v7.7.0.1.
> (require typed-racket/utils/prefab-c)         
> (struct contributor (x) #:prefab)
> (struct person-contributor contributor (a b) #:prefab)                       
> (struct surname-contributor person-contributor (y z w) #:prefab)         
> (prefab-struct-key (surname-contributor 1 2 3 4 5 6))
'(surname-contributor person-contributor 2 contributor 1)
> (define v (surname-contributor 1 2 3 4 5 6))                             
> v
'#s((surname-contributor person-contributor 2 contributor 1) 1 2 3 4 5 6)
> (define c (prefab/c (prefab-struct-key v) any/c any/c any/c any/c any/c any/c ))
> c
(prefab/c
 '(surname-contributor person-contributor 2 contributor 1)
 '(#<flat-contract: any/c>
   #<flat-contract: any/c>
   #<flat-contract: any/c>
   #<flat-contract: any/c>
   #<flat-contract: any/c>
   #<flat-contract: any/c>)
 56)
> (contract c v 1 2)
; surname-contributor-ref: index too large
;   index: 3
;   maximum allowed index: 2
;   structure: '#s((surname-contributor person-contributor 2 contributor 1) 1 2
;     3 4 5 6)
; [,bt for context]
> ,bt
; surname-contributor-ref: index too large
;   index: 3
;   maximum allowed index: 2
;   structure: '#s((surname-contributor person-contributor 2 contributor 1) 1 2 3 4 5 6)
;   context...:
;    /home/samth/sw/plt/extra-pkgs/typed-racket/typed-racket-lib/typed-racket/utils/prefab-c.rkt:86:4: for-loop
;    /home/samth/sw/plt/extra-pkgs/typed-racket/typed-racket-lib/typed-racket/utils/prefab-c.rkt:118:4
;    /home/samth/sw/plt/racket/collects/racket/contract/combinator.rkt:201:4
samth commented 4 years ago

Ok, the problem is that prefab/c is just wrong for prefab structs with parent structs. It assumes that the accessor procedure created via struct-type-info will work for all fields, but in fact it needs more complicated handling to support getting all the accessors up the hierarchy.