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

A memory read overrun issue in s_to_n32_unsafe.cc #215

Closed
mlite opened this Issue Sep 3, 2018 · 10 comments

Comments

Projects
None yet
3 participants
@mlite
Copy link

mlite commented Sep 3, 2018

This is the runtime error msg caused by the overrun.

DTS_MSG: Stensal C/C++ DTS detected a fatal program error!
DTS_MSG: Continuing the execution will cause unexpected behaviors, abort!
DTS_MSG: Reading 1 bytes at 0xffffc7dc will read undefined values.
DTS_MSG: Diagnostic information:

- The object to-be-read (start:0xffffc6dc, size:256 bytes) is allocated at
-     file:/home/sbuilder/workspace/re2c/re2c/src/test/s_to_n32_unsafe/test.cc::39, 10
-  0xffffc6dc               0xffffc7db
-  +------------------------+
-  | the object  to-be-read |......
-  +------------------------+
-                            ^~~~~~~~~~
-        the read starts at 0xffffc7dc that is right after the object end.
- Stack trace (most recent call first):
-[1]  file:/home/sbuilder/workspace/re2c/re2c/src/util/s_to_n32_unsafe.cc::28, 9
-[2]  file:/home/sbuilder/workspace/re2c/re2c/src/test/s_to_n32_unsafe/test.cc::50, 9
-[3]  file:/home/sbuilder/workspace/re2c/re2c/src/test/s_to_n32_unsafe/test.cc::85, 15
-[4]  file:/home/sbuilder/workspace/re2c/re2c/src/test/s_to_n32_unsafe/test.cc::101, 12
-[5]  file:/musl-1.1.10/src/env/__libc_start_main.c::168, 11
@skvadrik

This comment has been minimized.

Copy link
Owner

skvadrik commented Sep 4, 2018

Do you have the .re file which caused the failure?

@trofi

This comment has been minimized.

Copy link
Contributor

trofi commented Sep 4, 2018

Looks like it's caused by the test itself: re2c/src/test/s_to_n32_unsafe/test.cc. I haven't found plausible call path yet but a few use-of-uninitialized-value reports are detectable by clang's fsanitize=memory:

$ ./configure CC=clang CXX=clang++ CFLAGS="-fsanitize=memory" CXXFLAGS="-fsanitize=memory"
...
$ make check VERBOSE=1
...
FAIL: run_tests.sh
SKIP: testrange
PASS: testston32unsafe
PASS: testvertovernum
==================================
   re2c 1.1.1: ./test-suite.log
==================================

# TOTAL: 4
# PASS:  2
# SKIP:  1
# XFAIL: 0
# FAIL:  1
# XPASS: 0
# ERROR: 0

.. contents:: :depth: 2

FAIL: run_tests.sh
==================

Running in 8 thread(s)
FAIL       01_recognizing_integers.i.re
FAIL       config/cond_set/1_1_3.ci.re
FAIL       unicode_group_Cc.8--encoding-policy(fail).re
FAIL       posix_captures/implicit_grouping1.i--posix-captures.re
FAIL       range_dot.x.re
FAIL       parse_date.g.re
...
FAIL       parse_date.db.re
FAIL       unicode_group_No.u--encoding-policy(substitute).re
FAIL       posix_captures/gor3.i--posix-captures.re
Error: 1409 out 1409 tests failed.
FAIL run_tests.sh (exit status: 1)

SKIP: testrange
===============

==5661==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x49403d in re2c::Range::ran(unsigned int, unsigned int) (/home/slyfox/dev/git/re2c/re2c/testrange+0x49403d)
    #1 0x49390a in re2c::Range::append(re2c::Range**&, unsigned int, unsigned int) (/home/slyfox/dev/git/re2c/re2c/testrange+0x49390a)
    #2 0x491838 in main (/home/slyfox/dev/git/re2c/re2c/testrange+0x491838)
    #3 0x7fe65459579a in __libc_start_main /usr/src/debug/sys-libs/glibc-2.28/glibc-2.28/csu/../csu/libc-start.c:308:16
    #4 0x41b419 in _start (/home/slyfox/dev/git/re2c/re2c/testrange+0x41b419)

SUMMARY: MemorySanitizer: use-of-uninitialized-value (/home/slyfox/dev/git/re2c/re2c/testrange+0x49403d) in re2c::Range::ran(unsigned int, unsigned int)
Exiting
SKIP testrange (exit status: 77)

============================================================================
Testsuite summary for re2c 1.1.1
============================================================================

I'll try to add travis presubmit for a few sanitizers.

@skvadrik

This comment has been minimized.

Copy link
Owner

skvadrik commented Sep 4, 2018

Eh, I didn't read the bug report properly.

Valgrind also reports the error:

$ valgrind --track-origins=yes ./testston32unsafe 
==30881== Memcheck, a memory error detector
==30881== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==30881== Using Valgrind-3.14.0.GIT and LibVEX; rerun with -h for copyright info
==30881== Command: ./testston32unsafe
==30881== 
==30881== Conditional jump or move depends on uninitialised value(s)
==30881==    at 0x108C15: s_to_i32_unsafe(char const*, char const*, int&) (s_to_n32_unsafe.cc:28)
==30881==    by 0x108B56: re2c_test::test_i(long) (test.cc:50)
==30881==    by 0x1087BB: test (test.cc:85)
==30881==    by 0x1087BB: main (test.cc:101)
==30881==  Uninitialised value was created by a stack allocation
==30881==    at 0x108AB0: re2c_test::test_i(long) (test.cc:38)

trofi added a commit to trofi/re2c that referenced this issue Sep 4, 2018

__alltest.sh: add clang's -fsanitize=memory flavour
Bug: skvadrik#215
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>

trofi added a commit to trofi/re2c that referenced this issue Sep 4, 2018

.travis.yml: add sanitizers into test matrix
Bug: skvadrik#215
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>

skvadrik added a commit that referenced this issue Sep 4, 2018

Fixed bug #215 "A memory read overrun issue in s_to_n32_unsafe.cc".
The error was in the code of the test itself: the special case of zero
wasn't handled correctrly by the function that prepares input data for
the test. As a result, zero-length input string was passed to the test,
which is unexpected: the tested function is an "unsafe" one (as the
name suggests) and is meant to be used on an already validated input.

trofi added a commit to trofi/re2c that referenced this issue Sep 4, 2018

.travis.yml: add sanitizers into test matrix
Bug: skvadrik#215
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
@skvadrik

This comment has been minimized.

Copy link
Owner

skvadrik commented Sep 4, 2018

The error has been fixed (at least the one I can reproduce with Valgrind), see a439ca0?diff=unified.

@mlite, can you confirm the fix with your analyzer?

@trofi, thank you! :)

trofi added a commit to trofi/re2c that referenced this issue Sep 4, 2018

.travis.yml: add sanitizers into test matrix
Bug: skvadrik#215
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
@mlite

This comment has been minimized.

Copy link
Author

mlite commented Sep 5, 2018

the culprit is overflow-1.re. The memory read overrun is fixed, but I get stack overflow. What is the purpose of this test? to overflow the call stack?

@skvadrik

This comment has been minimized.

Copy link
Owner

skvadrik commented Sep 5, 2018

the culprit is overflow-1.re. The memory read overrun is fixed, but I get stack overflow.

It's a different bug. Can you open a new issue and specify what function overflowed? There is a number of recursive functions in re2c, and if the default stack size on your plaftorm is small (compared to that on the platforms where we test re2c), than it is quite possible that one of the recursive functions exhausted the stack.

What is the purpose of this test? to overflow the call stack?

No, actually it's to overflow re2c lexer buffer with an unexpectedly long lexeme: re2c used to crash at some point, but now it prints an error message.

What platform are you running re2c on? (My guess is, windows: I don't have it and the only kind of testing for windows is done by running Mingw-compiled re2c in Wine.)

@skvadrik

This comment has been minimized.

Copy link
Owner

skvadrik commented Sep 5, 2018

My guess is, windows

Eh, again I'm wrong. Your stacktrace shows this:

file:/musl-1.1.10/src/env/__libc_start_main.c
@mlite

This comment has been minimized.

Copy link
Author

mlite commented Sep 6, 2018

It's Linux. This issue is fixed. I will try it with a larger stack size.

@mlite mlite closed this Sep 6, 2018

trofi added a commit to trofi/re2c that referenced this issue Oct 21, 2018

.travis.yml: add sanitizers into test matrix
Bug: skvadrik#215
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
@mlite

This comment has been minimized.

Copy link
Author

mlite commented Jan 13, 2019

In case you are interested in what tool I used. I just released the tool at https://stensal.com. It's called Stensal SDK. It's free for personal use.

@skvadrik

This comment has been minimized.

Copy link
Owner

skvadrik commented Jan 13, 2019

@mlite , thanks !

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