MSVC x86_64 buffer overrun in error reporting (Regression between 3.3.5 and 3.3.6) #2046

asottile · 2016-04-24T20:22:19Z

This originally comes from sass/libsass-python#149

I'm having a hard time wrapping my head around exactly why this happens, but I'll post what I've got so far :)

This commit seems to cause the issue I'm seeing: 527f3a8 (#2025)
Checking out the revision before it (f8cad4e) seems to succeed.

My little test harness

You can view the code here if that's easier: https://github.com/asottile/libsass/commit/f40ae24025234b73ca86adece62dec0e35884eb1

main.cpp

#include <sass/context.h>
#include <iostream>

int main() {
    std::cout << "Making data context" << std::endl;
    struct Sass_Data_Context* context = sass_make_data_context(sass_copy_c_string(""));
    std::cout << "Compiling data context" << std::endl;
    sass_compile_data_context(context);
    std::cout << "Getting output context" << std::endl;
    struct Sass_Context* ctx = sass_data_context_get_context(context);
    std::cout << "Printing error status" << std::endl;
    std::cout << sass_context_get_error_status(ctx) << std::endl;
    std::cout << "Printing error message" << std::endl;
    std::cout << sass_context_get_error_message(ctx) << std::endl;
    sass_delete_data_context(context);

    return 0;
}

test.bat

"D:\Programs\VS2015\VC\BIN\amd64\cl.exe" ^
    /I.\include -ID:\Programs\VS2015\VC\INCLUDE ^
    /c /nologo /W3 /WX- /GL /DNDEBUG -O2 /Oi /Zi /EHsc /MT ^
    src/*.c
"D:\Programs\VS2015\VC\BIN\amd64\cl.exe" ^
    /I.\include ^
    "-ID:\Programs\VS2015\VC\INCLUDE" ^
    "-ID:\Programs\VS2015\VC\ATLMFC\INCLUDE" ^
    "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.10240.0\ucrt" ^
    "-IC:\Program Files (x86)\Windows Kits\NETFXSDK\4.6.1\include\um" ^
    "-IC:\Program Files (x86)\Windows Kits\8.1\include\\shared" ^
    "-IC:\Program Files (x86)\Windows Kits\8.1\include\\um" ^
    "-IC:\Program Files (x86)\Windows Kits\8.1\include\\winrt" ^
    /c /nologo /W3 /WX- /GL /DNDEBUG -O2 /Oi /Zi /EHsc /MT ^
    ./main.cpp src/*.cpp 
"D:\Programs\VS2015\VC\BIN\amd64\link.exe" ^
    "-LIBPATH:D:\Programs\VS2015\VC\LIB\amd64" ^
    "-LIBPATH:D:\Programs\VS2015\VC\ATLMFC\LIB\amd64" ^
    "-LIBPATH:C:\Program Files (x86)\Windows Kits\10\lib\10.0.10240.0\ucrt\x64" ^
    "-LIBPATH:C:\Program Files (x86)\Windows Kits\NETFXSDK\4.6.1\lib\um\x64" ^
    "-LIBPATH:C:\Program Files (x86)\Windows Kits\8.1\lib\winv6.3\um\x64" ^
    "-INCREMENTAL:NO" "-OUT:main.exe" "-Debug" "-nologo" "-LTCG" ^
    *.obj
main.exe

Output at `f8cad4e`

C:\Users\Anthony\Desktop\git\libsass-python\libsass>"D:\Programs\VS2015\VC\BIN\amd64\cl.exe"     /I.\include -ID:\Programs\VS2015\VC\INCLUDE     /c /nologo /W3 /WX- /GL /DNDEBUG -O2 /Oi /Zi /EHsc /MT     src/*.c
c99func.c
cencode.c

C:\Users\Anthony\Desktop\git\libsass-python\libsass>"D:\Programs\VS2015\VC\BIN\amd64\cl.exe"     /I.\include     "-ID:\Programs\VS2015\VC\INCLUDE"     "-ID:\Programs\VS2015\VC\ATLMFC\INCLUDE"     "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.10240.0\ucrt"     "-IC:\Program Files (x86)\Windows Kits\NETFXSDK\4.6.1\include\um"     "-IC:\Program Files (x86)\Windows Kits\8.1\include\\shared"     "-IC:\Program Files (x86)\Windows Kits\8.1\include\\um"     "-IC:\Program Files (x86)\Windows Kits\8.1\include\\winrt"     /c /nologo /W3 /WX- /GL /DNDEBUG -O2 /Oi /Zi /EHsc /MT     ./main.cpp src/*.cpp
main.cpp
ast.cpp
base64vlq.cpp
bind.cpp
src/bind.cpp(175): warning C4267: 'initializing': conversion from 'size_t' to 'int', possible loss of data
color_maps.cpp
constants.cpp
context.cpp
cssize.cpp
emitter.cpp
environment.cpp
error_handling.cpp
eval.cpp
expand.cpp
extend.cpp
file.cpp
functions.cpp
inspect.cpp
json.cpp
lexer.cpp
listize.cpp
Compiling...
memory_manager.cpp
node.cpp
output.cpp
parser.cpp
plugins.cpp
position.cpp
prelexer.cpp
remove_placeholders.cpp
sass.cpp
sass2scss.cpp
sass_context.cpp
sass_functions.cpp
sass_util.cpp
sass_values.cpp
source_map.cpp
to_c.cpp
to_value.cpp
units.cpp
utf8_string.cpp
util.cpp
Compiling...
values.cpp

C:\Users\Anthony\Desktop\git\libsass-python\libsass>"D:\Programs\VS2015\VC\BIN\amd64\link.exe"     "-LIBPATH:D:\Programs\VS2015\VC\LIB\amd64"     "-LIBPATH:D:\Programs\VS2015\VC\ATLMFC\LIB\amd64"     "-LIBPATH:C:\Program Files (x86)\Windows Kits\10\lib\10.0.10240.0\ucrt\x64"     "-LIBPATH:C:\Program Files (x86)\Windows Kits\NETFXSDK\4.6.1\lib\um\x64"     "-LIBPATH:C:\Program Files (x86)\Windows Kits\8.1\lib\winv6.3\um\x64"     "-INCREMENTAL:NO" "-OUT:main.exe" "-Debug" "-nologo" "-LTCG"     *.obj
Generating code
Finished generating code

C:\Users\Anthony\Desktop\git\libsass-python\libsass>main.exe
Making data context
Compiling data context
Getting output context
Printing error status
3
Printing error message
Internal Error: Data context created with empty source string

Output at `527f3a8`

C:\Users\Anthony\Desktop\git\libsass-python\libsass>"D:\Programs\VS2015\VC\BIN\amd64\cl.exe"     /I.\include -ID:\Programs\VS2015\VC\INCLUDE     /c /nologo /W3 /WX- /GL /DNDEBUG -O2 /Oi /Zi /EHsc /MT     src/*.c
c99func.c
cencode.c

C:\Users\Anthony\Desktop\git\libsass-python\libsass>"D:\Programs\VS2015\VC\BIN\amd64\cl.exe"     /I.\include     "-ID:\Programs\VS2015\VC\INCLUDE"     "-ID:\Programs\VS2015\VC\ATLMFC\INCLUDE"     "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.10240.0\ucrt"     "-IC:\Program Files (x86)\Windows Kits\NETFXSDK\4.6.1\include\um"     "-IC:\Program Files (x86)\Windows Kits\8.1\include\\shared"     "-IC:\Program Files (x86)\Windows Kits\8.1\include\\um"     "-IC:\Program Files (x86)\Windows Kits\8.1\include\\winrt"     /c /nologo /W3 /WX- /GL /DNDEBUG -O2 /Oi /Zi /EHsc /MT     ./main.cpp src/*.cpp
main.cpp
ast.cpp
base64vlq.cpp
bind.cpp
src/bind.cpp(175): warning C4267: 'initializing': conversion from 'size_t' to 'int', possible loss of data
color_maps.cpp
constants.cpp
context.cpp
cssize.cpp
emitter.cpp
environment.cpp
error_handling.cpp
eval.cpp
expand.cpp
extend.cpp
file.cpp
functions.cpp
inspect.cpp
json.cpp
lexer.cpp
listize.cpp
Compiling...
memory_manager.cpp
node.cpp
output.cpp
parser.cpp
plugins.cpp
position.cpp
prelexer.cpp
remove_placeholders.cpp
sass.cpp
sass2scss.cpp
sass_context.cpp
sass_functions.cpp
sass_util.cpp
sass_values.cpp
source_map.cpp
to_c.cpp
to_value.cpp
units.cpp
utf8_string.cpp
util.cpp
Compiling...
values.cpp

C:\Users\Anthony\Desktop\git\libsass-python\libsass>"D:\Programs\VS2015\VC\BIN\amd64\link.exe"     "-LIBPATH:D:\Programs\VS2015\VC\LIB\amd64"     "-LIBPATH:D:\Programs\VS2015\VC\ATLMFC\LIB\amd64"     "-LIBPATH:C:\Program Files (x86)\Windows Kits\10\lib\10.0.10240.0\ucrt\x64"     "-LIBPATH:C:\Program Files (x86)\Windows Kits\NETFXSDK\4.6.1\lib\um\x64"     "-LIBPATH:C:\Program Files (x86)\Windows Kits\8.1\lib\winv6.3\um\x64"     "-INCREMENTAL:NO" "-OUT:main.exe" "-Debug" "-nologo" "-LTCG"     *.obj
Generating code
Finished generating code

C:\Users\Anthony\Desktop\git\libsass-python\libsass>main.exe
Making data context

Attaching debugger on failure gives the following message:

Unhandled exception at 0x00007FF77A8F8814 in main.exe: Stack cookie instrumentation code detected a stack-based buffer overrun.

And drops a breakpoint at: https://github.com/sass/libsass/blob/527f3a8/src/sass_context.cpp#L143

The text was updated successfully, but these errors were encountered:

mgreter · 2016-04-24T22:10:44Z

First 👏 for that bug report, pretty much on spot as we want them. If you take the time to replace D:\Programs with ProgramFiles(x86) and install visual studio in a standard path (Microsoft Visual Studio 12.0 AFAIK) next time, I will officially nominate it for the bug report of the year 😀

Already getting late here, but will take a quick peek ...

mgreter · 2016-04-24T22:12:56Z

Seems removing /GL (program optimization) makes the issue go away ...
To clarify, I'm getting segfault when optimization is enabled .. ok, see you have the same.
Since you say it came with the newly added try, I suspect stack unwinding problems.
This is often related to compiler options, but too soon to say anything definite.

asottile · 2016-04-24T22:24:42Z

Will do and report back. I'm out of space on my SSD so I'll probably need to spin up a VM

asottile · 2016-04-24T22:27:21Z

Changing to /GR does the trick! -- I think I can fix that up in the flags I pass to setuptools (python).

Thanks for the tip!

asottile · 2016-04-24T22:31:52Z

Hmmm... this is slightly difficult to do since those are some defaults that are difficult to override -- any idea why this commit breaks /GL ?

mgreter · 2016-04-24T22:33:56Z

Yeah, because memory is probably more scrambled, so it is probably also easier to get segfaults via dangling pointers or buffer overruns etc. But I can already report that it is a dreaded, hard to spot "issue" we are actually not facing for the first time. Fix is underway, will write a bit more in the PR.

asottile · 2016-04-24T22:34:18Z

<3 you're the best

mgreter · 2016-04-24T22:43:41Z

If you like to give it a thought yourself, this is the offending line:

sass_copy_c_string(msg_stream.str().c_str())

mgreter · 2016-04-24T23:12:36Z

See #2048 for proposed fix. Thanks for the report!

asottile · 2016-04-24T23:14:44Z

I do remember something similar to this in some code I wrote a long while back -- I thought the standard protected against the string temporary being collected but perhaps not.

This patch seems to make the code succeed:

diff --git a/src/sass_context.cpp b/src/sass_context.cpp
index e3f34af..9105866 100644
--- a/src/sass_context.cpp
+++ b/src/sass_context.cpp
@@ -140,7 +140,8 @@ extern "C" {
       json_append_member(json_err, "message", json_mkstring(e.what()));
       json_append_member(json_err, "formatted", json_mkstring(msg_stream.str().c_str()));
       try { c_ctx->error_json = json_stringify(json_err, "  "); } catch(...) {}
-      c_ctx->error_message = sass_copy_c_string(msg_stream.str().c_str());
+      std::string s = msg_stream.str();
+      c_ctx->error_message = sass_copy_c_string(s.c_str());
       c_ctx->error_text = sass_copy_c_string(e.what());
       c_ctx->error_status = 3;
       c_ctx->output_string = 0;

asottile · 2016-04-24T23:15:02Z

oh sweet, we came to the same conclusion :D

mgreter · 2016-04-24T23:15:48Z

Yep, saw that one before with the memory management, where destructor was called in an odd ordering that lead to basically the same issue (same compiler).

mgreter · 2016-04-24T23:17:48Z

Ah, now I remember, that is exactly the reason (AFAIR) why we have the SASS_MEMORY_NEW macro for allocation of new objects. That was this PR: #1462 ... not 100% sure, but my gut says they have both the same common root.

xzyfer · 2016-04-25T03:23:39Z

Nice catch y'all
On 25 Apr 2016 9:29 AM, "Marcel Greter" notifications@github.com wrote:

Closed #2046 #2046 via #2048
#2048.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#2046 (comment)

asottile mentioned this issue Apr 24, 2016

Update libsass to fix materializecss crash sass/libsass-python#149

Closed

mgreter added the Bug - Confirmed label Apr 24, 2016

asottile closed this as completed Apr 24, 2016

mgreter reopened this Apr 24, 2016

mgreter mentioned this issue Apr 24, 2016

Fix issue when using intermediate string returns #2048

Merged

mgreter added the Dev - PR Ready label Apr 24, 2016

mgreter added this to the 3.3.7 milestone Apr 24, 2016

mgreter self-assigned this Apr 24, 2016

mgreter closed this as completed in #2048 Apr 24, 2016

mgreter mentioned this issue Apr 24, 2016

Segfault compiling specially crafted sass file into CSS #1962

Closed

asottile mentioned this issue Oct 14, 2016

@extend causing a coredump sass/libsass-python#165

Closed

xzyfer modified the milestones: 3.3.7, 3.4 Oct 20, 2016

asottile mentioned this issue Oct 24, 2016

Stack overflow / buffer underrun in 3.4.0RC1 in debug mode (MSVC x86_64) #2221

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MSVC x86_64 buffer overrun in error reporting (Regression between 3.3.5 and 3.3.6) #2046

MSVC x86_64 buffer overrun in error reporting (Regression between 3.3.5 and 3.3.6) #2046

asottile commented Apr 24, 2016

mgreter commented Apr 24, 2016

mgreter commented Apr 24, 2016 •

edited

Loading

asottile commented Apr 24, 2016

asottile commented Apr 24, 2016

asottile commented Apr 24, 2016

mgreter commented Apr 24, 2016

asottile commented Apr 24, 2016

mgreter commented Apr 24, 2016

mgreter commented Apr 24, 2016

asottile commented Apr 24, 2016

asottile commented Apr 24, 2016

mgreter commented Apr 24, 2016

mgreter commented Apr 24, 2016 •

edited

Loading

xzyfer commented Apr 25, 2016

MSVC x86_64 buffer overrun in error reporting (Regression between 3.3.5 and 3.3.6) #2046

MSVC x86_64 buffer overrun in error reporting (Regression between 3.3.5 and 3.3.6) #2046

Comments

asottile commented Apr 24, 2016

My little test harness

Output at f8cad4e

Output at 527f3a8

mgreter commented Apr 24, 2016

mgreter commented Apr 24, 2016 • edited Loading

asottile commented Apr 24, 2016

asottile commented Apr 24, 2016

asottile commented Apr 24, 2016

mgreter commented Apr 24, 2016

asottile commented Apr 24, 2016

mgreter commented Apr 24, 2016

mgreter commented Apr 24, 2016

asottile commented Apr 24, 2016

asottile commented Apr 24, 2016

mgreter commented Apr 24, 2016

mgreter commented Apr 24, 2016 • edited Loading

xzyfer commented Apr 25, 2016

Output at `f8cad4e`

Output at `527f3a8`

mgreter commented Apr 24, 2016 •

edited

Loading

mgreter commented Apr 24, 2016 •

edited

Loading