Merge pull request sass#3022 from mgreter/bugfix/3019-extend-error-sr…

…c-lifetime Extend `error_src` lifetime on c-api context
saper · Oct 29, 2019 · a089875 · a089875
2 parents f7567ea + ed91ca4
commit a089875
Show file tree

Hide file tree

Showing 6 changed files with 16 additions and 5 deletions.
diff --git a/docs/api-context-internal.md b/docs/api-context-internal.md
@@ -117,7 +117,7 @@ struct Sass_Context : Sass_Options
   char* error_file;
   size_t error_line;
   size_t error_column;
-  const char* error_src;
+  char* error_src;
 
   // report imported files
   char** included_files;

diff --git a/docs/api-context.md b/docs/api-context.md
@@ -124,6 +124,7 @@ char* error_message;
 char* error_file;
 size_t error_line;
 size_t error_column;
+char* error_src;
 ```
 ```C
 // report imported files
@@ -202,6 +203,7 @@ const char* sass_context_get_error_json (struct Sass_Context* ctx);
 const char* sass_context_get_error_text (struct Sass_Context* ctx);
 const char* sass_context_get_error_message (struct Sass_Context* ctx);
 const char* sass_context_get_error_file (struct Sass_Context* ctx);
+const char* sass_context_get_error_src (struct Sass_Context* ctx);
 size_t sass_context_get_error_line (struct Sass_Context* ctx);
 size_t sass_context_get_error_column (struct Sass_Context* ctx);
 const char* sass_context_get_source_map_string (struct Sass_Context* ctx);
@@ -221,6 +223,7 @@ char* sass_context_take_error_json (struct Sass_Context* ctx);
 char* sass_context_take_error_text (struct Sass_Context* ctx);
 char* sass_context_take_error_message (struct Sass_Context* ctx);
 char* sass_context_take_error_file (struct Sass_Context* ctx);
+char* sass_context_take_error_src (struct Sass_Context* ctx);
 char* sass_context_take_output_string (struct Sass_Context* ctx);
 char* sass_context_take_source_map_string (struct Sass_Context* ctx);
 ```

diff --git a/include/sass/context.h b/include/sass/context.h
@@ -134,6 +134,7 @@ ADDAPI char* ADDCALL sass_context_take_error_json (struct Sass_Context* ctx);
 ADDAPI char* ADDCALL sass_context_take_error_text (struct Sass_Context* ctx);
 ADDAPI char* ADDCALL sass_context_take_error_message (struct Sass_Context* ctx);
 ADDAPI char* ADDCALL sass_context_take_error_file (struct Sass_Context* ctx);
+ADDAPI char* ADDCALL sass_context_take_error_src (struct Sass_Context* ctx);
 ADDAPI char* ADDCALL sass_context_take_output_string (struct Sass_Context* ctx);
 ADDAPI char* ADDCALL sass_context_take_source_map_string (struct Sass_Context* ctx);
 ADDAPI char** ADDCALL sass_context_take_included_files (struct Sass_Context* ctx);

diff --git a/src/parser.cpp b/src/parser.cpp
@@ -2894,6 +2894,10 @@ namespace Sass {
     Position p(pos.line ? pos : before_token);
     ParserState pstate(path, source, p, Offset(0, 0));
     // `pstate.src` may not outlive stack unwind so we must copy it.
+    // This is needed since we often parse dynamically generated code,
+    // e.g. for interpolations, and we normally don't want to keep this
+    // memory around after we parsed the AST tree successfully. Only on
+    // errors we want to preserve them for better error reporting.
     char *src_copy = sass_copy_c_string(pstate.src);
     pstate.src = src_copy;
     traces.push_back(Backtrace(pstate));

diff --git a/src/sass_context.cpp b/src/sass_context.cpp
@@ -122,7 +122,7 @@ namespace Sass {
       c_ctx->error_file = sass_copy_c_string(e.pstate.path);
       c_ctx->error_line = e.pstate.line + 1;
       c_ctx->error_column = e.pstate.column + 1;
-      c_ctx->error_src = e.pstate.src;
+      c_ctx->error_src = sass_copy_c_string(e.pstate.src);
       c_ctx->output_string = 0;
       c_ctx->source_map_string = 0;
       json_delete(json_err);
@@ -277,8 +277,8 @@ extern "C" {
       c_ctx->error_message = 0;
       c_ctx->error_status = 0;
       // reset error position
-      c_ctx->error_src = 0;
       c_ctx->error_file = 0;
+      c_ctx->error_src = 0;
       c_ctx->error_line = std::string::npos;
       c_ctx->error_column = std::string::npos;
 
@@ -542,6 +542,7 @@ extern "C" {
     if (ctx->error_text)        free(ctx->error_text);
     if (ctx->error_json)        free(ctx->error_json);
     if (ctx->error_file)        free(ctx->error_file);
+    if (ctx->error_src)         free(ctx->error_src);
     free_string_array(ctx->included_files);
     // play safe and reset properties
     ctx->output_string = 0;
@@ -550,6 +551,7 @@ extern "C" {
     ctx->error_text = 0;
     ctx->error_json = 0;
     ctx->error_file = 0;
+    ctx->error_src = 0;
     ctx->included_files = 0;
     // debug leaked memory
     #ifdef DEBUG_SHARED_PTR
@@ -648,9 +650,9 @@ extern "C" {
   IMPLEMENT_SASS_CONTEXT_GETTER(const char*, error_message);
   IMPLEMENT_SASS_CONTEXT_GETTER(const char*, error_text);
   IMPLEMENT_SASS_CONTEXT_GETTER(const char*, error_file);
+  IMPLEMENT_SASS_CONTEXT_GETTER(const char*, error_src);
   IMPLEMENT_SASS_CONTEXT_GETTER(size_t, error_line);
   IMPLEMENT_SASS_CONTEXT_GETTER(size_t, error_column);
-  IMPLEMENT_SASS_CONTEXT_GETTER(const char*, error_src);
   IMPLEMENT_SASS_CONTEXT_GETTER(const char*, output_string);
   IMPLEMENT_SASS_CONTEXT_GETTER(const char*, source_map_string);
   IMPLEMENT_SASS_CONTEXT_GETTER(char**, included_files);
@@ -660,6 +662,7 @@ extern "C" {
   IMPLEMENT_SASS_CONTEXT_TAKER(char*, error_message);
   IMPLEMENT_SASS_CONTEXT_TAKER(char*, error_text);
   IMPLEMENT_SASS_CONTEXT_TAKER(char*, error_file);
+  IMPLEMENT_SASS_CONTEXT_TAKER(char*, error_src);
   IMPLEMENT_SASS_CONTEXT_TAKER(char*, output_string);
   IMPLEMENT_SASS_CONTEXT_TAKER(char*, source_map_string);
   IMPLEMENT_SASS_CONTEXT_TAKER(char**, included_files);

diff --git a/src/sass_context.hpp b/src/sass_context.hpp
@@ -90,7 +90,7 @@ struct Sass_Context : Sass_Options
   char* error_file;
   size_t error_line;
   size_t error_column;
-  const char* error_src;
+  char* error_src;
 
   // report imported files
   char** included_files;