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

Missing tests #2

Open
6 of 8 tasks
aaaaaa123456789 opened this issue Jul 28, 2018 · 6 comments
Open
6 of 8 tasks

Missing tests #2

aaaaaa123456789 opened this issue Jul 28, 2018 · 6 comments

Comments

@aaaaaa123456789
Copy link

aaaaaa123456789 commented Jul 28, 2018

While I don't have any reason to suspect the instructions are executing incorrectly, the following cases are not tested at all:

  • ifle/ifge
  • xordata with size 0
  • fillbyte/fillhalfword/fillword with count 0
  • fillhalfword/fillword with overflowing counts (e.g., 0x80000001)
  • zero-initialization of registers #0 and #1
  • the various operand combinations for shift instructions (since it is the only case where the first byte doesn't fully determine the instruction's length and its operands)
  • overflowing the buffer with bufchar/bufstring/bufnumber (e.g., putting a (valid) bufstring instruction inside an infinite loop)
  • bufchar 0 followed by another bufchar/bufstring/bufnumber (the null character must not cause the rest of the output to be suppressed)

I'll comment if I find any others missing.

@fstirlitz
Copy link
Owner

  • fillhalfword/fillword with overflowing counts (e.g., 0x80000001)

I don't think I handle overflowing file positions very well, especially that it might be hard to test without putting resource pressure on the testing environment. (Also, I thought about allowing any position the platform's off_t can handle and only triggering an overflow error on pos/pushpos, but that would be against the spec, it seems.).

  • overflowing the buffer with bufchar/bufstring/bufnumber (e.g., putting a (valid) bufstring instruction inside an infinite loop)

The buffer is dynamically-allocated; overflow should not be possible, only memory exhaustion. I remember attempting to create a test case for that, but I found it difficult to actually trigger memory exhaustion without the executed opcodes/backward branches limits clocking out first. I might need to establish additional upper limits on buffer length/stack depth anyway.

  • bufchar 0 followed by another bufchar/bufstring/bufnumber (the null character must not cause the rest of the output to be suppressed)

Actually, attempting to print an ASCII NUL makes the command-line front-end trigger a fatal error:

$ ./runtest --verbose tests/bufchar_nul.test 
=== Compiling ===
=== Disassembling ===
=== Disassembly ===
                        bufchar            0x00000000
                        bufstring          str_0000000d
                        printbuf           
                        exit               #0
str_0000000d:
                        string             "hello"

=== Compiling again ===
=== Verifying ===
=== Running ===
=== Standard error ===
[  0/000001] 00000000  a200000000              bufchar           0x00000000
                                               ------
             0000000d                          string            "hello"
[  0/000002] 00000005  a00d000000              bufstring         0x0000000d
                                               ------
[  0/000003] 0000000a  a6                      printbuf          
./bsp: fatal error: attempt to print out an illegal character 0x00
  traceback after 3 ops executed:
      0. pc=0x0000000a  stk#=0  [/tmp/bsptest-rh_k8ndk/bsp0]
./bsp: keeping scratch file '/tmp/bsptest-rh_k8ndk/output.1802.XXX1VpVKQ'
./bsp: /tmp/bsptest-rh_k8ndk/bsp0: 3 instructions executed
./bsp: /tmp/bsptest-rh_k8ndk/bsp0: 0 backward jumps

=== Expected output ===

=== Expected final data ===

=== Expected exit code: 0 ===
=== Unexpected fatal error ===

This is because I wanted to prevent rogue patch scripts from reprogramming the user's terminal with control characters. Strictly speaking, this is in violation of the spec (which seems to encourage filtering characters out instead), but I didn't think it was worth it: the terminal output code is already quite complicated, and why would you ever want to legitimately print out control characters to the terminal anyway? (Colour output, I guess. But the spec doesn't mandate supporting ANSI escapes, nor does it offer any way to check if they are available/will be cleanly filtered out, so it won't be portable between different BSP implementations. Or, in fact, a single implementation running on different terminals.)

@aaaaaa123456789
Copy link
Author

Regarding the last point, the spec mandates accepting control characters, not printing them; it only requires a subset of printable ASCII to actually be printable. An implementation that accepts control characters in strings but refuses to print them (or only prints newlines) is compliant.

An engine incapable of handling the full Unicode character set may choose to use a reduced character set and replace the remaining characters with a suitable substitution character; however, an engine must at least support the Latin letters (A-Z, a-z), digits (0-9), spaces, and the following punctuation characters: '-,.;:#%&!?/()[].

@fstirlitz
Copy link
Owner

That paragraph doesn't specify if aborting the script upon encountering an unsupported character is acceptable. The previous one only allows it in the case of attempting to print invalid UTF-8.

But given that the actual contents of the buffer cannot possibly influence further execution (beyond raising a fatal error, as discussed), there seems to be little point in testing this.

And tangentially, strictly speaking the spec suggests that (potential) fatal errors due to bad encoding should only be raised upon an attempt to print the string, not when putting it in the buffer. So this:

	bufstring bad_utf8
	clearbuf
	exit 0
bad_utf8:
	db 0x80, 0

should exit cleanly. But both my implementation and yours generate a fatal error.

@aaaaaa123456789
Copy link
Author

Interesting. I'll have to amend the specification, if anything to allow that behavior (as it seems silly to require that example to work).

@aaaaaa123456789
Copy link
Author

I actually made a pull on my own repo regarding this: aaaaaa123456789/bsp#1

Opinions are appreciated.

@aaaaaa123456789
Copy link
Author

Are bit shifts with overly long counts tested? I mean something like this:

  set #1, 0x21
  shiftright #1, #1

The result should be 0x10. (The spec clearly says that shift-by-register only uses the lowest five bits of the shift count and ignores the rest.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants