diff --git a/.gitattributes b/.gitattributes index 284508182..3a5d9bf5a 100644 --- a/.gitattributes +++ b/.gitattributes @@ -25,3 +25,8 @@ bugs/** -text # The VB6 template `bug_repro.mjs new --with-vb6` copies into a reproducer's vb6/ # is CRLF in every checkout, as the bugs/** files it becomes are, and VB6 needs it. test/repro-templates/vb6/** -text + +# The PngDump modules `bug_repro.mjs new --with-images` copies into a reproducer are CRLF +# in every checkout for the same reason: PngDump.bas is VB6's, and the copy is compared +# with the template line by line. +test/repro-templates/png/** -text diff --git a/.github/actions/run-gates/action.yml b/.github/actions/run-gates/action.yml index fd3d42325..92e15dfc8 100644 --- a/.github/actions/run-gates/action.yml +++ b/.github/actions/run-gates/action.yml @@ -82,6 +82,14 @@ runs: - name: Unit-test the date formatter (test/strftime.test.mjs) shell: bash run: node --test test/strftime.test.mjs + # Unit tests for the pictures of a bug reproducer: PNG decoding, comparing + # and the side-by-side image (scripts/lib/png.mjs), the files behind + # repro.json's images and expect.imagesDiffer, and bug_repro.mjs's own + # refusals over fixtures. Starts no IDE and no VB6. No browser, no built + # tree, about 4 s. + - name: Unit-test the bug reproducers' pictures (test/png.test.mjs) + shell: bash + run: node --test test/png.test.mjs # check_examples.mjs's probes: batching, crash isolation through a fake # lane, the canaries and the fence classifier. check_examples.mjs needs a # twinBASIC install, which CI has not; its probes need none. diff --git a/BUGS-TO-REPORT.md b/BUGS-TO-REPORT.md index e90bf4170..ef05bce1c 100644 --- a/BUGS-TO-REPORT.md +++ b/BUGS-TO-REPORT.md @@ -60,7 +60,12 @@ How the four duties above fit it: the narrowed steps. - **Expected behavior** says what should happen instead, and why: what VBA or VB6 does, what the documentation says, or what the IDE does in the neighbouring case. -- **Screenshots** is left out when there are none. +- **Screenshots** is left out when there are none. A graphical defect's section says what + each panel of its comparison shows and embeds the picture, as + `![](bugs//images/-compare.png)`, so that it shows in this file; + `bug_repro.mjs file` rewrites the path to `images/-compare.png` in `REPORT.md`. A + GitHub issue cannot reach a path in the repository, so when the entry is pasted into one, + the picture is dragged into the issue in place of that line. - **Desktop** gives the build as `BETA `, the build it was last reproduced on. - **Additional context** gives what did not reproduce it, the severity, and the builds it was checked on besides the one above. @@ -86,8 +91,15 @@ kebab-case name for the bug, and its **To Reproduce** names the project file: | `bugs//src/` | the project's exported source tree: `Settings`, `Sources/` and the rest | yes, byte for byte | | `bugs//.twinproj` | the project file, packed from `src/` | yes | | `bugs//.zip` | the `.twinproj` zipped, because a GitHub issue does not accept a `.twinproj` attachment, with any file `repro.json`'s `attach` names, such as a `.twinpack` | no | -| `bugs//vb6/` | optional: a VB6 project, to show what VB6 does where the entry compares it with twinBASIC: `Probe.vbp` and its `.bas`, `.cls` and `.frm` files, sources only, never an exe or an output | yes, byte for byte | +| `bugs//vb6/` | optional: a VB6 project, to show what VB6 does where the entry compares it with twinBASIC: `Probe.vbp` and its `.bas`, `.cls`, `.frm` and `.ctl` files (with their `.frx` and `.ctx`), sources only, never an exe or an output | yes, byte for byte | | `bugs//-vb6.zip` | the source files of `vb6/` zipped, to attach beside the other zip; written only when `vb6/` exists | no | +| `bugs//images/` | optional: the pictures of a graphical defect, `-tb.png` (from `run`), `-vb6.png` (from `vb6`) and `-compare.png`, which shows both and their difference; the zip holds them all | yes | + +**A graphical defect carries pictures.** Its `repro.json` names them under `images`, the +`PngDump` modules from `test/repro-templates/png/` save them (`new --with-images` copies them), +and its entry embeds each `-compare.png` under **Screenshots**: what twinBASIC drew beside +what VB6 drew, the pixels that differ in red. `test/png.test.mjs` fails when an entry or a filed +`REPORT.md` does not embed one. `expect.imagesDiffer` lets `verify` judge by them. `scripts/bug_repro.mjs` makes and checks them (the tool's page is [Tools and Scripts](docs/Documentation/Tools.md#bug-repro)): @@ -245,7 +257,7 @@ So exit 0 on a project with errors happens only for an error in code nothing cal Silence on stdout and stderr: measured on BETA 983 first, and on BETA 995 by redirecting the standard handles of the process to a file. The same redirection captures the compiler executable's own output (`twinBASIC_win32.exe settings ` wrote the whole `Settings` file), so the capture works. - + --- @@ -344,31 +356,32 @@ What does not reproduce it: a long input path to `export`, and any output folder *DEFERRED until after v1* **Describe the bug** -Given a file that is not a valid project, the compiler executable opens a modal message box (*invalid header* and *invalid file format* were both seen) and prints nothing more until it is closed. `export`, `settings` and `readme` each waited on the box indefinitely. Once the box is closed, `export` prints `WARNING: failed to parse project file, file may be corrupt`, then `... DONE`, and exits 0. +Given a file that is not a valid project, the compiler executable opens a modal message box and prints nothing more until it is closed. `export`, `settings` and `readme` each wait on the box indefinitely. Once the box is closed, `export` prints `WARNING: failed to parse project file, file may be corrupt`, then `... DONE`, and exits 0; `settings` and `readme` print an `ERROR:` line, and exit 0 as well. **To Reproduce** Steps to reproduce the behavior: -1. Unzip `damaged-project-modal-box.zip` (it holds `damaged-project-modal-box.twinproj`, an ordinary valid project) and make damaged copies of it, in PowerShell: `$b = [IO.File]::ReadAllBytes("C:\p\damaged-project-modal-box.twinproj")`, then `$b[0] = $b[0] -bxor 0xFF; [IO.File]::WriteAllBytes("C:\p\firstbyte.twinproj", $b)` for one with its first byte changed, and `[IO.File]::WriteAllBytes("C:\p\cut.twinproj", $b[0..([int]($b.Length / 2))])` for one cut off halfway. A 20-byte text file named `garbage.twinproj`, or an empty file, is damaged enough too. +1. Unzip `damaged-project-modal-box.zip`. It holds `damaged-project-modal-box.twinproj`, an ordinary valid project, and four damaged copies of it: `garbage.twinproj`, `empty.twinproj`, `firstbyte.twinproj` and `cut.twinproj`. They were made in PowerShell: `$b = [IO.File]::ReadAllBytes("C:\p\damaged-project-modal-box.twinproj")`, then `$b[0] = $b[0] -bxor 0xFF; [IO.File]::WriteAllBytes("C:\p\firstbyte.twinproj", $b)` for one with its first byte changed, and `[IO.File]::WriteAllBytes("C:\p\cut.twinproj", $b[0..([int]($b.Length / 2))])` for one cut off halfway. A 20-byte text file named `garbage.twinproj`, or an empty file, is damaged enough too. 2. Run `twinBASIC_win32.exe export C:\p\garbage.twinproj C:\p\out\`, on a desktop someone is watching. -3. See a message box and no further output. The command was still waiting when it was ended after 25 seconds. -4. Close the box. See `WARNING: failed to parse project file, file may be corrupt`, then `... DONE`, and exit code 0. For the project cut off halfway it also writes the one file it could read, `Settings`. -5. Run `settings` or `readme` on the same file: each waits on a box the same way. +3. See a message box titled `TWINBASIC ERROR`, saying `invalid file header: bad file format`, and no output after the `exporting from ... to ...` line. The command is still waiting when it is ended after 20 seconds. The empty file and the one with its first byte changed give the same box. +4. Close the box. See `WARNING: failed to parse project file, file may be corrupt`, then `... DONE`, and exit code 0, with no file written. +5. Run `settings` or `readme` on the same file: each waits on the same box, prints nothing until it is closed, then prints `ERROR: failed to parse project file, file may be corrupt or inaccessible` and exits 0. +6. Run `export` on the project cut off halfway. The box is titled `bad file format` and says `WARNING: failed to deserialize file system. Some files might be lost or truncated.` Once it is closed, `export` writes the one file it could read, `Sources\Startup.twin`, prints `[EXPORT] DONE:` for it and then `... DONE`, with no warning, and exits 0. `settings` on it waits on the same box, then prints `ERROR: failed to find file, file may be corrupt`; `readme` waits, then prints nothing. **Expected behavior** -No window opens from a command-line verb. The command prints the problem (`ERROR: failed to parse project file, file may be corrupt or inaccessible`, which it already prints when a folder is given where the project should be), ends `... FAILED`, and does not report `... DONE` for a file it could not read. +No window opens from a command-line verb. The command prints the problem (`ERROR: failed to parse project file, file may be corrupt or inaccessible`, which it already prints when a folder is given where the project should be), ends `... FAILED`, does not report `... DONE` for a file it could not read, and exits with a code other than 0. **Desktop:** - OS: Windows 10 Pro 22H2 (build 19045) - - twinBASIC compiler version: BETA 983 (the 20-byte text file was also seen on BETA 995) + - twinBASIC compiler version: BETA 995 **Additional context** -Severity: an unattended `export`, `settings` or `readme` never finishes, and once the box is closed `export` reports success, so a script that tests for `... DONE` is fooled as well. +Severity: an unattended `export`, `settings` or `readme` never finishes, and once the box is closed every one of them exits 0 and `export` reports success, so a script that tests the exit code or `... DONE` is fooled as well. -All four kinds of damaged input were tried on BETA 983: a 20-byte text file, an empty file, a real project with its first byte changed, and one cut off halfway. On BETA 995 a run with a 20-second limit found `export` and `settings` on the 20-byte text file still waiting at the limit; the other inputs returned inside it, and whether each opened a box was not checked. +All four kinds of damaged input open the box under all three commands, on BETA 995 and on BETA 983. The undamaged project exports at once, with no box. On BETA 983 `export` of the project cut off halfway wrote `Settings` rather than `Sources\Startup.twin`. What does not reproduce it: a folder given where the project should be. `settings` then prints `ERROR: failed to parse project file, file may be corrupt or inaccessible` and exits, with no box. - + --- @@ -854,7 +867,7 @@ What was tried: - An interface with three `[PreserveSig]` members declared in `IUnknown`'s order, `QueryInterface`, `AddRef` and `Release`, behaves the same way: `Set` succeeds, and the calls go to the wrong slots (one `AddRef` returned 0, and the next call crashed). - `Interface IUnk Extends stdole.IUnknown` with that identifier compiles, and `u.AddRef` is then reported as `TB5027 Unrecognized member 'AddRef' on type 'IUnk'`, as it is for `stdole.IUnknown` itself, which has no members that twinBASIC code can call. - + --- @@ -897,7 +910,7 @@ What does not reproduce it: a statement with no arguments (`o.Fail3`, `o.Hello`, The second call resembles VB's rule for `o.Member(args)` on a property that returns an object or a collection: read the property with no arguments, then apply the arguments to the result. It is applied after a failure of any call that has arguments. The single run of `Hello` in the raw `Invoke` case is also at odds with the COM contract, which gives `DISP_E_BADPARAMCOUNT` for too many arguments without running the member; it is left out of this entry. - + --- @@ -932,7 +945,7 @@ What does not reproduce it: `CallByName` with `vbGet` or `vbLet` against `DISP_E A separate behaviour, in its own entry, repeats a failed late-bound statement that passes arguments as a property read: see the entry "A late-bound call that passes arguments and fails is issued a second time, without them". The two are not the same: that one is for calls with arguments and needs no `DISP_E_MEMBERNOTFOUND`, and this one is `CallByName` with or without arguments. - + --- @@ -1091,7 +1104,7 @@ Severity: low for a program that passes all the arguments; for one that does not Related and also different from VB6: `Err.Raise 65536` is accepted, and `Err.Number` is 65536, where VB6 raises error 5. `Err.Raise 0` raises error 5 in both. An explicit empty string for the source or the description gives an empty `Source` or `Description`, in VB6 as well. `Err.HelpContext` is 0 in twinBASIC, and VB6 sets it to 1000000 plus the number for a `Raise` without one (`1000005` for 5). - + --- @@ -1128,7 +1141,7 @@ The two symptoms are one bug because the object is the same: `ObjPtr` of what `G What does not reproduce it: a fresh thread (`GetErrorInfo` returns `S_FALSE`); a caller that makes the call through an interface whose methods are declared `[PreserveSig]` and reads the slot itself, where the first `GetErrorInfo` returns the values and the second returns `S_FALSE`; the second `GetErrorInfo` after the object has been read from the slot (it returns `S_FALSE`, so `GetErrorInfo` does empty the slot). Severity: low. The slot is not a documented twinBASIC interface, but a program that reads error information with the COM functions, or a library that does, sees values that change under it, and a program cannot rely on the description of a failure that came with no error information. - + --- @@ -1171,7 +1184,7 @@ Severity: low in practice, since a program rarely advises a null sink, but a COM What was tried: an object that is not a sink gives an ordinary error (`E_NOINTERFACE`, `&H80004002`, see the entry about `Advise` and `Unadvise` error codes), so only a null pointer crashes. The address of the crash is the same in the standalone probe and in this project. - + --- @@ -1207,7 +1220,7 @@ Steps to reproduce the behavior: Severity: low. A client that tests for `CONNECT_E_CANNOTCONNECT` to tell a refused sink from other failures never sees it, and a client that releases a connection twice, or with a wrong cookie, is told it worked. What was tried: a sink that is an ordinary twinBASIC class gives the same `E_NOINTERFACE` whether it has no members or implements `IDispatch` (the outgoing interface's identifier is generated for each build, so a twinBASIC class cannot answer for it). Nothing is connected afterwards, and no event reaches the sink. Calling `Unadvise` a second time with the cookie of the failed `Advise`, which is 0, also returns without an error. - + --- @@ -1245,3 +1258,457 @@ VB6 has no comparison: a project cannot declare an `Interface`, and the interfac +--- + +## Clear All Breakpoints is undone by restarting the compiler, unless the project was saved in between + +**Describe the bug** +Breakpoints removed with **Debug > Clear All Breakpoints** come back when the compiler restarts. After **Restart the compiler** on the toolbar the margin shows them again, as they were just before they were cleared, and the next run stops at them. A breakpoint removed with F9 stays removed, and saving the project between the two keeps the effect of **Clear All Breakpoints**. + +**To Reproduce** +Steps to reproduce the behavior: +1. Open `clear-all-breakpoints-restart.twinproj` (attached as `clear-all-breakpoints-restart.zip`) and open `Startup.twin`. `Main` prints three lines: + ``` + Public Sub Main() + Debug.Print "main start" + Debug.Print "the breakpoint's line" ' BREAK + Debug.Print "main end" + End Sub + ``` +2. Put the cursor on the line marked `BREAK` and press F9. A breakpoint appears in the margin. +3. Choose **Debug > Clear All Breakpoints** (Ctrl+Shift+F9). The breakpoint goes. +4. Click **Restart the compiler** on the toolbar, and wait for the compile to end. The breakpoint is back in the margin. +5. Press F5. The run stops at the breakpoint, having printed only `main start`. + +**Expected behavior** +Breakpoints cleared with **Clear All Breakpoints** stay cleared, as a breakpoint removed with F9 does: after the restart the margin shows none, and F5 prints all three lines. + +**Desktop:** + - OS: Windows 10 Pro 22H2 (build 19045) + - twinBASIC compiler version: BETA 995 + +**Additional context** +Also on BETA 987, identically. + +What does not reproduce it: removing the breakpoint with F9 instead, which stays removed across the restart; and saving the project (Ctrl+S) after **Clear All Breakpoints** and before the restart, which keeps them cleared. A breakpoint saved with the project survives a restart, as it should. Not tried: whether the IDE's other ways of starting a new compiler, such as switching between win32 and win64, do the same. + +Where it seems to come from, in BETA 995's `ide/main.js`: F9 records each change in `g_SessionFilesystemTransactions`, which the IDE replays into a new compiler and which a save empties; **Clear All Breakpoints** (`tbDebug_BreakpointsClear`) clears the breakpoints in the compiler and records nothing there, so the replay sets them again. + +Severity: breakpoints a person has removed stop the program again after a restart, with nothing to connect the two. + + + +--- + +## A watch on a variable of a user-defined type fails with a codegen error, and the Debug Console reports a linker error at every stop + +**Describe the bug** +While the debugger is stopped, a watch on a variable of a user-defined type cannot be evaluated. **Watches** shows `(compile error: codegen error; check for compilation errors)`, of type `ERROR`, and the Debug Console prints `[LINKER] compilation (codegen) error detected in 'Startup.{temp_procedure}' at line #1`. The linker error comes again at every later stop while the watch exists, and `? p` in the Debug Console fails the same way. **Variables** shows the same variable and its fields without trouble, and a watch on one of its fields works. + +**To Reproduce** +Steps to reproduce the behavior: +1. Open `udt-watch-codegen-error.twinproj` (attached as `udt-watch-codegen-error.zip`). `Startup.twin` declares a type, and `Main` passes the line marked `BREAK` twice: + ``` + Private Type Point + X As Long + Y As Long + End Type + + Public Sub Main() + Dim p As Point + Dim i As Long + p.X = 7 + For i = 1 To 2 + Debug.Print "pass " & i ' BREAK + Next + End Sub + ``` +2. Put a breakpoint on the line marked `BREAK` (F9) and press F5. The run stops there, and **Variables** shows `p` as `{user defined type, 8 bytes}`, with `X` and `Y`. +3. Add a watch on `p` (**Debug > Add Watch...**, or the plus sign in **Watches**). The watch shows `(compile error: codegen error; check for compilation errors)`, of type `ERROR`, and the Debug Console prints `[LINKER] compilation (codegen) error detected in 'Startup.{temp_procedure}' at line #1`. +4. Press F5. The run stops at the breakpoint again, and the Debug Console prints the linker error again. +5. Type `? p` in the Debug Console and press Enter. It fails with the same error, and the Debug Console prints the same linker line. + +**Expected behavior** +The watch shows `p` as **Variables** does, `{user defined type, 8 bytes}` opening to its fields, as VBA's Watch window shows a variable of a user-defined type. An expression that cannot be shown, such as `? p`, which has no single value to print, is refused with a message about the expression. A linker error about generated code reads as though the project had failed to build, which it has not. + +**Desktop:** + - OS: Windows 10 Pro 22H2 (build 19045) + - twinBASIC compiler version: BETA 995 + +**Additional context** +Also on BETA 987, identically. + +What does not reproduce it: a watch on a field, `p.X`, which shows `7`, of type `Variant [Long]`; and **Variables**, which shows `p` and its fields. That type suggests a watch is evaluated as a `Variant`, which cannot hold a value of a user-defined type. Seen with a `Private Type` declared in the module; other declarations of the type were not tried. + +Severity: a watch on a structure is unusable, and while it exists every stop adds a linker error to the Debug Console. + + + +--- + +## After Break Into Code stops in DoEvents, evaluating anything ends the program with an access violation + +**Describe the bug** +**Break Into Code** (Ctrl+Break), pressed while a program is in `DoEvents`, stops inside the VB package rather than in the program: the IDE opens the package's `IdleMessageLoopBreak.twin` at `IdleMessageLoopBreakpoint`, which **Call Stack** shows above the program's procedure. Evaluating anything at that stop ends the program with a native access violation: a line in the Debug Console, even `? 1`, or any watch, which the IDE evaluates at every stop. The IDE then stays in break mode, with the yellow arrow on the package's line and **Stop** and the step commands enabled, although the program has ended. + +**To Reproduce** +Steps to reproduce the behavior: +1. Open `break-in-doevents-evaluate-crash.twinproj` (attached as `break-in-doevents-evaluate-crash.zip`). `Main` loops on `DoEvents`: + ``` + Public Sub Main() + Do + DoEvents + Loop + End Sub + ``` +2. Press F5, then Ctrl+Break (**Run > Break**). The IDE opens `IdleMessageLoopBreak.twin` under `Packages/VB/Sources/SUPPORT`, stopped in `IdleMessageLoopBreakpoint`, and **Call Stack** shows `IdleMessageLoopBreakpoint` above `Main`. +3. Type `? 1` in the Debug Console and press Enter. The program ends, and the Debug Console prints: + ``` + (runtime error -2147467259: NATIVE EXCEPTION: ACCESS_VIOLATION) + 0001