Skip to content

Merge 1.12 ure fix - #7

Closed
jepler wants to merge 4 commits into
tannewt:merge_1.12from
jepler:merge_1.12_ure_fix
Closed

jepler wants to merge 4 commits into
tannewt:merge_1.12from
jepler:merge_1.12_ure_fix

Conversation

@jepler

@jepler jepler commented Apr 28, 2021

Copy link
Copy Markdown
Collaborator

@tannewt I can re-roll without the #if defined(DEBUG_COMPILECODE) if you prefer.

The meat is this commit:

re1.5: Fix handling of escapes within character classes

In CircuitPython, we've added support for escaped characters in
character classes, like `r'[\r\n\t]'`.  On the micropython side
they added a simpler bit of code to skip over escapes within
character classes so that `[a\\b]' worked.  However, this very test
case was handled incorrectly by our own added code.

Basically, when a character class contains a *non-range*, the same
character of the pattern is parsed twice, to give the lower and upper
bounds of the range.  However, the code we added caused the regular
expression location to be advanced when the first backlash was encountered,
making r'[a\\b]' be parsed more like '[a\\-\b]' which was nonsense.

Track where parsing one character-or-range started (the new local variable
"b") and if a non-range is encountered, set the parsing position back to
where we started, which moves back over the backslash if necessary.

Also, add a check (in two places) for the degenerate regular expression
ending with a backslash inside of a character class, such as '[\\'
This fixes the bug, while preserving our new behavior.

tannewt and others added 4 commits April 28, 2021 10:57
This allows you to build and test a standalone commandline program
down in extmod/re1.5, e.g.,

```
$ gcc -std=c11 -DDEBUG_COMPILECODE compilecode.c dumpcode.c && ./a.out '[\\-a]*'
 0: rsplit 5 (3)
 2: any
 3: jmp 0 (-5)
 5: save 0
 7: split 15 (6)
 9: class 1 0x5c-0x61
13: jmp 7 (-8)
15: save 1
17: match
Bytes: 18, insts: 9
```

It doesn't affect the code that's included when building CircuitPython.
The "\t" escape code for tab wasn't in the little table.

"\x" was translated to a literal backslash, but this is not standard
behavior.
In CircuitPython, we've added support for escaped characters in
character classes, like `r'[\r\n\t]'`.  On the micropython side
they added a simpler bit of code to skip over escapes within
character classes so that `[a\\b]' worked.  However, this very test
case was handled incorrectly by our own added code.

Basically, when a character class contains a *non-range*, the same
character of the pattern is parsed twice, to give the lower and upper
bounds of the range.  However, the code we added caused the regular
expression location to be advanced when the first backlash was encountered,
making r'[a\\b]' be parsed more like '[a\\-\b]' which was nonsense.

Track where parsing one character-or-range started (the new local variable
"b") and if a non-range is encountered, set the parsing position back to
where we started, which moves back over the backslash if necessary.

Also, add a check (in two places) for the degenerate regular expression
ending with a backslash inside of a character class, such as '[\\'
This fixes the bug, while preserving our new behavior.
@tannewt
tannewt force-pushed the merge_1.12 branch 2 times, most recently from 023a56f to bf0746e Compare April 28, 2021 22:00
@jepler jepler closed this Apr 29, 2021
tannewt pushed a commit that referenced this pull request Jun 24, 2021
asan considers that memcmp(p, q, N) is permitted to access N bytes at each
of p and q, even for values of p and q that have a difference earlier.
Accessing additional values is frequently done in practice, reading 4 or
more bytes from each input at a time for efficiency, so when completing
"non_exist<TAB>" in the repl, this causes a diagnostic:

    ==16938==ERROR: AddressSanitizer: global-buffer-overflow on
    address 0x555555cd8dc8 at pc 0x7ffff726457b bp 0x7fffffffda20 sp 0x7fff
    READ of size 9 at 0x555555cd8dc8 thread T0
        #0 0x7ffff726457a  (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xb857a)
        #1 0x555555b0e82a in mp_repl_autocomplete ../../py/repl.c:301
        #2 0x555555c89585 in readline_process_char ../../lib/mp-readline/re
        #3 0x555555c8ac6e in readline ../../lib/mp-readline/readline.c:513
        #4 0x555555b8dcbd in do_repl /home/jepler/src/micropython/ports/uni
        #5 0x555555b90859 in main_ /home/jepler/src/micropython/ports/unix/
        #6 0x555555b90a3a in main /home/jepler/src/micropython/ports/unix/m
        #7 0x7ffff619a09a in __libc_start_main ../csu/libc-start.c:308
        #8 0x55555595fd69 in _start (/home/jepler/src/micropython/ports/uni

    0x555555cd8dc8 is located 0 bytes to the right of global variable
    'import_str' defined in '../../py/repl.c:285:23' (0x555555cd8dc0) of
    size 8
      'import_str' is ascii string 'import '

Signed-off-by: Jeff Epler <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants