Many test report the wrong expect value

[Turro75]

Please have a look at my pull request 06dabf9 where I PR the right expect values of many test (till b.n -20)

[urish]

Thanks, I'll have a look!

[urish]

This makes me think: if we create an abstract async interface for the tests, then we could possibly create some kind of adapter to run them against the actual chip!

Similar to what you did in #13, but automatic instead of manual. This can later be extended to even more complex test sequences (e.g. testing NVIC operation).

Something along the lines of:

    it('should execute `ands r5, r0` instruction', async () => {
      const cpu = new CPUDebugger();
      await cpu.setPC(PC, 0x20000000);
      await cpu.writeMemory(0x20000000, opcodeANDS(r5, r0));
      await cpu.setRegister(r5, 0xffff0000);
      await cpu.setRegister(r6, 0xf00fffff);
      await cpu.executeInstruction();
      const registers = await cpu.readRegisters();
      expect(registers[r5]).toEqual(0xf00f0000);
      expect(registers.N).toEqual(true);
      expect(registers.Z).toEqual(false);
      expect(registers.C).toEqual(false);
      expect(registers.V).toEqual(false);
    });

Then you could swap the implementation of CPUDebugger with one that actually speaks to the Pi Pico through the GDB interface to run all the tests against the physical device. This could be done through an environment variable, for example.

It would let us quickly verify that all tests indeed pass on the physical device.

Thoughts?

[Turro75]

Hi Uri, I reviewed my PR and I realized that I made a lot or errors (sorry for that) since in many cases I set pc=0x20000000 while the test still writes the opcode at 0x10000000 so many test failed due to this. This reduced the number of failed test to 3. Then I realized that on a real pico I cannot write in the flash 0x10000000 and in the rp2040 class I cannot write opcodes at 0x20000000 so the only way I see is the one You suggested. Anyway CPUDebugger must have extended ranges on flash to allow the use of 0x20000000 as opcode store memory.

[urish]

This reduced the number of failed test to 3.

That's still much!

Then I realized that on a real pico I cannot write in the flash 0x10000000 and in the rp2040 class I cannot write opcodes at 0x20000000 so the only way I see is the one You suggested.

Why not? What if you use rp2040.writeUint16(0x20000000, opcode); ?

[Turro75]

.... yes it works ... (typing while wearing donkey ears)

[Turro75]

2 test that fails: add r1, sp, #16 add sp, r8

[urish]

GDB Test driver output:

PS C:\P\rp2040js> npm test --runInBand

> rp2040js@0.3.5 test C:\P\rp2040js
> jest

 PASS  src/utils/assembler.spec.ts
 PASS  src/rp2040.spec.ts
 FAIL  src/instructions.spec.ts (24.24 s)
  ● Cortex-M0+ Instruction Set › should execute `adcs r5, r4` instruction

    Invalid singleStep response: OK

      142 |     const response = await this.sendCommand('vCont;s:1;c');
      143 |     if (!response.startsWith('T')) {
    > 144 |       throw new Error(`Invalid singleStep response: ${response}`);
          |             ^
      145 |     }
      146 |   }
      147 | 

      at GDBClient.<anonymous> (src/utils/gdbclient.ts:144:13)
      at fulfilled (src/utils/gdbclient.ts:5:58)

  ● Cortex-M0+ Instruction Set › should execute a `add r1, sp, #4` instruction

    expect(received).toEqual(expected) // deep equality

    Expected: 85
    Received: 84

      157 |     await cpu.singleStep();
      158 |     const registers = await cpu.readRegisters();
    > 159 |     expect(registers.sp).toEqual(0x55);
          |                          ^
      160 |     expect(registers.r1).toEqual(0x65);
      161 |   });
      162 | 

      at src/instructions.spec.ts:159:26
      at fulfilled (src/instructions.spec.ts:5:58)

  ● Cortex-M0+ Instruction Set › should execute `add r1, ip` instruction

    expect(received).toEqual(expected) // deep equality

    Expected: false
    Received: true

      209 |     const registers = await cpu.readRegisters();
      210 |     expect(registers.r1).toEqual(110);
    > 211 |     expect(registers.N).toEqual(false);
          |                         ^
      212 |     expect(registers.Z).toEqual(false);
      213 |     expect(registers.C).toEqual(false);
      214 |     expect(registers.V).toEqual(false);

      at src/instructions.spec.ts:211:25
      at fulfilled (src/instructions.spec.ts:5:58)

  ● Cortex-M0+ Instruction Set › should execute `add sp, r8` instruction and not update the flags

    expect(received).toEqual(expected) // deep equality

    Expected: 537067537
    Received: 537067536

      221 |     await cpu.singleStep();
      222 |     const registers = await cpu.readRegisters();
    > 223 |     expect(registers.sp).toEqual(0x20030011);
          |                          ^
      224 |     expect(registers.Z).toEqual(true); // assert it didn't update the flags
      225 |   });
      226 | 

      at src/instructions.spec.ts:223:26
      at fulfilled (src/instructions.spec.ts:5:58)

  ● Cortex-M0+ Instruction Set › should execute a `pop pc, {r4, r5, r6}` instruction

    expect(received).toEqual(expected) // deep equality

    Expected: 64
    Received: 721455168

      575 |     expect(registers.sp).toEqual(RAM_START_ADDRESS + 0x100);
      576 |     // assert that the values of r4, r5, r6, pc were poped from the stack correctly
    > 577 |     expect(registers.r4).toEqual(0x40);
          |                          ^
      578 |     expect(registers.r5).toEqual(0x50);
      579 |     expect(registers.r6).toEqual(0x60);
      580 |     expect(registers.pc).toEqual(0x42);

      at src/instructions.spec.ts:577:26
      at fulfilled (src/instructions.spec.ts:5:58)

  ● Cortex-M0+ Instruction Set › should execute an `ldr r3, [r2, #24]` instruction

    expect(received).toEqual(expected) // deep equality

    Expected: 85
    Received: 268435797

      667 |     await cpu.singleStep();
      668 |     const registers = await cpu.readRegisters();
    > 669 |     expect(registers.r3).toEqual(0x55);
          |                          ^
      670 |   });
      671 | 
      672 |   it('should execute an `ldr r3, [sp, #12]` instruction', async () => {

      at src/instructions.spec.ts:669:26
      at fulfilled (src/instructions.spec.ts:5:58)
          at runMicrotasks (<anonymous>)

  ● Cortex-M0+ Instruction Set › should execute an `ldr r3, [sp, #12]` instruction

    expect(received).toEqual(expected) // deep equality

    Expected: 85
    Received: 43605

      677 |     await cpu.singleStep();
      678 |     const registers = await cpu.readRegisters();
    > 679 |     expect(registers.r3).toEqual(0x55);
          |                          ^
      680 |   });
      681 | 
      682 |   it('should execute an `ldr r3, [r5, r6]` instruction', async () => {

      at src/instructions.spec.ts:679:26
      at fulfilled (src/instructions.spec.ts:5:58)
          at runMicrotasks (<anonymous>)

  ● Cortex-M0+ Instruction Set › should raise an SVCALL exception when `svc` instruction runs

    Invalid singleStep response: OK

      142 |     const response = await this.sendCommand('vCont;s:1;c');
      143 |     if (!response.startsWith('T')) {
    > 144 |       throw new Error(`Invalid singleStep response: ${response}`);
          |             ^
      145 |     }
      146 |   }
      147 |

      at GDBClient.<anonymous> (src/utils/gdbclient.ts:144:13)
      at fulfilled (src/utils/gdbclient.ts:5:58)

Test Suites: 1 failed, 2 passed, 3 total
Tests:       8 failed, 164 passed, 172 total
Snapshots:   0 total
Time:        24.904 s, estimated 331 s
Ran all test suites.
npm ERR! Test failed.  See above for more details.

[urish]

add r1, sp, #16 add sp, r8

Regarding these tests, see section A5.1.3 in the ARMv6-M Architecture Reference Manual:

R13<1:0> definition Bits [1:0] of R13 are treated as Should Be Zero or Preserved (SBZP). Writing a non-zero value to bits [1:0] results in UNPREDICTABLE behavior. Reading bits [1:0] returns zero.

I have opened a new issue for this, #17

[Turro75]

Well done, at least 1 bug found after 4hrs of heavy work streaming

[urish]

Yes, and also these ones:

• d2363552f41b2046ea07e68631670833fd550d5e (missing breaks in switch statement in exceptionReturn())
• ee6326fbc53092dc067503c9fb06a43c3bb28fa3

So total of 3 bugs!

I tried running the problematic SysTick example again this morning, and this time it worked flawlessly! So it must have been one of those bugs that caused PM to go on and then never off

[Turro75]

So having a trustful asm can lead in straight peripherals implementation, You are creating a solid base for many other arm devices

[urish]

That's true!

For instance, SAMD21 is also based on ARM Cortex M0+