search

simonowen / samdisk

A portable disk image utility, specialising in copy-protected PC-compatible formats.
http://simonowen.com/samdisk
MIT License
86 stars 11 forks source link

std::bad_alloc can happen in scan_bitstream_mfm_fm() #10

Closed shattered closed 6 years ago

shattered commented 6 years ago

I'm using a 96tpi drive to read disks written on a 48tpi drive - occasionally, data from adjacent even track are picked up on odd track and this confuses scan_bitstream_mfm_fm() -- for whatever reason, next_idam_distance / next_dam_distance is computed as zero, and then next_idam_align / next_dam_bytes become negative... Boom, allocation failure of -1 bytes :)

My workaround is 

auto extent_bytes = std::max(0, read_gap2 ? next_dam_bytes : next_idam_bytes);

And offending track looks like this after it's applied:

[my80-021.scp]
Cyl 11 Head 0:
sync lost at offset 24458 (24458)
sync lost at offset 26313 (26313)
sync lost at offset 26941 (26941)
sync lost at offset 27803 (27803)
sync lost at offset 29407 (29407)
sync lost at offset 30096 (30096)
sync lost at offset 32933 (32933)
sync lost at offset 33349 (33349)
sync lost at offset 34354 (34354)
sync lost at offset 36178 (36178)
sync lost at offset 37013 (37013)
sync lost at offset 39884 (39884)
sync lost at offset 47924 (47924)
sync lost at offset 52307 (52307)
sync lost at offset 52958 (52958)
sync lost at offset 53368 (53368)
sync lost at offset 54002 (54002)
sync lost at offset 54453 (54453)
sync lost at offset 55682 (55682)
sync lost at offset 59939 (59939)
sync lost at offset 61139 (61139)
sync lost at offset 63952 (63952)
sync lost at offset 65050 (65050)
sync lost at offset 65345 (65345)
sync lost at offset 66055 (66055)
sync lost at offset 66376 (66376)
sync lost at offset 69493 (69493)
sync lost at offset 70353 (70353)
sync lost at offset 70983 (70983)
sync lost at offset 74575 (74575)
sync lost at offset 75346 (75346)
sync lost at offset 76459 (76459)
sync lost at offset 77233 (77233)
sync lost at offset 77826 (77826)
sync lost at offset 78954 (78954)
sync lost at offset 79840 (79840)
sync lost at offset 80144 (80144)
sync lost at offset 81576 (81576)
sync lost at offset 82276 (82276)
sync lost at offset 82864 (82864)
sync lost at offset 83465 (83465)
sync lost at offset 85680 (85680)
sync lost at offset 694 (694)
sync lost at offset 1996 (1996)
sync lost at offset 16580 (16580)
sync lost at offset 17845 (17845)
sync lost at offset 32449 (32449)
sync lost at offset 33725 (33725)
sync lost at offset 158723 (158723)
sync lost at offset 159820 (159820)
sync lost at offset 160114 (160114)
sync lost at offset 1233 (1233)
sync lost at offset 1666 (1666)
sync lost at offset 4870 (4870)
sync lost at offset 5242 (5242)
sync lost at offset 5912 (5912)
sync lost at offset 16246 (16246)
sync lost at offset 17909 (17909)
sync lost at offset 20855 (20855)
sync lost at offset 21358 (21358)
sync lost at offset 23097 (23097)
sync lost at offset 23925 (23925)
sync lost at offset 24236 (24236)
sync lost at offset 24514 (24514)
sync lost at offset 25606 (25606)
sync lost at offset 25926 (25926)
sync lost at offset 26388 (26388)
sync lost at offset 26981 (26981)
sync lost at offset 27668 (27668)
sync lost at offset 28457 (28457)
sync lost at offset 29945 (29945)
sync lost at offset 30491 (30491)
sync lost at offset 31368 (31368)
sync lost at offset 31724 (31724)
sync lost at offset 39462 (39462)
sync lost at offset 42591 (42591)
sync lost at offset 60533 (60533)
sync lost at offset 62026 (62026)
sync lost at offset 63308 (63308)
sync lost at offset 69708 (69708)
sync lost at offset 71344 (71344)
sync lost at offset 87695 (87695)
sync lost at offset 89567 (89567)
sync lost at offset 90347 (90347)
sync lost at offset 93540 (93540)
sync lost at offset 95646 (95646)
sync lost at offset 97833 (97833)
sync lost at offset 98425 (98425)
sync lost at offset 100390 (100390)
* DAM (am=251) at offset 11079 (11079)
* IDAM (id=3) at offset 20103 (20103)
Finding cyl 11 head 0 sector 3:
sync lost at offset 1268 (1268)
sync lost at offset 1661 (1661)
sync lost at offset 5265 (5265)
sync lost at offset 16217 (16217)
sync lost at offset 17879 (17879)
sync lost at offset 21312 (21312)
sync lost at offset 23033 (23033)
sync lost at offset 23860 (23860)
sync lost at offset 24572 (24572)
sync lost at offset 24979 (24979)
sync lost at offset 25516 (25516)
sync lost at offset 26281 (26281)
sync lost at offset 27522 (27522)
sync lost at offset 28284 (28284)
sync lost at offset 28970 (28970)
sync lost at offset 29712 (29712)
sync lost at offset 30229 (30229)
sync lost at offset 31116 (31116)
sync lost at offset 31398 (31398)
sync lost at offset 33187 (33187)
sync lost at offset 35080 (35080)
sync lost at offset 38743 (38743)
sync lost at offset 41738 (41738)
sync lost at offset 58871 (58871)
sync lost at offset 60287 (60287)
sync lost at offset 60789 (60789)
sync lost at offset 61493 (61493)
sync lost at offset 67580 (67580)
sync lost at offset 69147 (69147)
sync lost at offset 75096 (75096)
sync lost at offset 84777 (84777)
sync lost at offset 86558 (86558)
sync lost at offset 87308 (87308)
sync lost at offset 88107 (88107)
sync lost at offset 89334 (89334)
sync lost at offset 90350 (90350)
sync lost at offset 92375 (92375)
sync lost at offset 93133 (93133)
sync lost at offset 94464 (94464)
sync lost at offset 95050 (95050)
sync lost at offset 97006 (97006)
* DAM (am=251) at offset 11060 (11060)
sync lost at offset 1239 (1239)
sync lost at offset 1677 (1677)
sync lost at offset 4887 (4887)
sync lost at offset 5259 (5259)
sync lost at offset 5930 (5930)
sync lost at offset 16268 (16268)
sync lost at offset 17934 (17934)
sync lost at offset 20944 (20944)
sync lost at offset 21391 (21391)
sync lost at offset 23144 (23144)
sync lost at offset 23618 (23618)
sync lost at offset 23984 (23984)
sync lost at offset 24303 (24303)
sync lost at offset 24581 (24581)
sync lost at offset 25690 (25690)
sync lost at offset 26482 (26482)
sync lost at offset 27079 (27079)
sync lost at offset 27781 (27781)
sync lost at offset 28589 (28589)
sync lost at offset 30141 (30141)
sync lost at offset 30706 (30706)
sync lost at offset 31986 (31986)
sync lost at offset 62165 (62165)
sync lost at offset 65074 (65074)
sync lost at offset 71718 (71718)
sync lost at offset 92507 (92507)
sync lost at offset 93325 (93325)
sync lost at offset 98773 (98773)
sync lost at offset 101180 (101180)
sync lost at offset 101783 (101783)
sync lost at offset 103751 (103751)
* IDAM (id=2) at offset 10309 (10309)
* DAM (am=251) at offset 11098 (11098)
* IDAM (id=3) at offset 20134 (20134)
Finding cyl 11 head 0 sector 2:
Finding cyl 11 head 0 sector 3:
* IDAM (id=2) at offset 10309 (10309)
* DAM (am=251) at offset 11098 (11098)
* IDAM (id=3) at offset 20134 (20134)
Finding cyl 11 head 0 sector 2:
Finding cyl 11 head 0 sector 3:
Warning: late track start on cyl 11 head 0 may indicate missing first sector
300Kbps MFM,  2 sectors,  512 bytes/sector, c=5:
 11.0  2[dc,z] 3[nd] 
         6485: 644 1256 [5229]
simonowen commented 6 years ago

Your work-around avoids the crash, but the zero size generates the 'z' flag in the scan output, which means a DAM was found but no sector data is stored. That's considered an unusual condition so it's shown in yellow. It may be better to skip the data match as though no DAM was found. Though, since the odd tracks aren't really going to be used then anything is better than a crash!

The underlying issue is related to the scan combining the headers for all revolutions into a single track view. I think there might have been a problem if a header wasn't seen on the first revolution, or its bitstream position changed too much, and that is more likely to happen on the fringes of odd-numbered 48tpi tracks when read in a 96tpi drive.

I've seen this happen too, and started modifying BitstreamDecoder.cpp to fix it. That's what the conditional code comment ("disabled until header/data matching enhancements are complete") refers to. Hopefully I still have the stashed code changes and a sample disk to reproduce it.

simonowen commented 6 years ago

I ran into this again today, and found that the distance calculation was wrong when calculating the forward distance back to the same field. 14c0d22892f8a29c4785e9e6dac415d253320b80 should resolve it.

I made the same change to the Agat code, so you might want to check it's still working as expected for your needs. It should now give a 'nd' (no-data) indicator instead of the 'z' (zero-bytes) in your example above.

shattered commented 6 years ago

Agat code works fine - thanks.