From: Wolfgang Bumiller <wry.git@bumiller.com>
Date: Sun, 23 Jul 2017 08:06:51 +0000 (+0200)
Subject: move more parser code to c++, fix crashes with gcc
X-Git-Tag: xonotic-v0.8.5~34
X-Git-Url: https://git.rm.cloudns.org/?a=commitdiff_plain;h=047ecd426f4068566a2db97a894d00d98c66f345;p=xonotic%2Fgmqcc.git

move more parser code to c++, fix crashes with gcc

we initialized the parser with malloc -> memset to zero ->
placement new. With gcc the latter caused the memset to be
optimized out, causing uninitialized value accesses.
---

diff --git a/gmqcc.h b/gmqcc.h
index 112628e..10b74f5 100644
--- a/gmqcc.h
+++ b/gmqcc.h
@@ -741,7 +741,6 @@ parser_t *parser_create(void);
 bool parser_compile_file(parser_t *parser, const char *);
 bool parser_compile_string(parser_t *parser, const char *, const char *, size_t);
 bool parser_finish(parser_t *parser, const char *);
-void parser_cleanup(parser_t *parser);
 
 /* ftepp.c */
 struct ftepp_t;
diff --git a/main.cpp b/main.cpp
index 449067d..76c1d31 100644
--- a/main.cpp
+++ b/main.cpp
@@ -3,6 +3,7 @@
 
 #include "gmqcc.h"
 #include "lexer.h"
+#include "parser.h"
 
 /* TODO: cleanup this whole file .. it's a fuckign mess */
 
@@ -736,7 +737,7 @@ cleanup:
     vec_free(ppems);
 
     if (!OPTS_OPTION_BOOL(OPTION_PP_ONLY))
-        if(parser) parser_cleanup(parser);
+        delete parser;
 
     /* free allocated option strings */
     for (itr = 0; itr < OPTION_COUNT; itr++)
diff --git a/parser.cpp b/parser.cpp
index 92434f0..1e15ba3 100644
--- a/parser.cpp
+++ b/parser.cpp
@@ -139,7 +139,7 @@ static ast_expression* parser_find_local(parser_t *parser, const char *name, siz
     hash = util_hthash(parser->htglobals, name);
 
     *isparam = false;
-    for (i = vec_size(parser->variables); i > upto;) {
+    for (i = parser->variables.size(); i > upto;) {
         --i;
         if ( (e = (ast_expression*)util_htgeth(parser->variables[i], name, hash)) )
             return e;
@@ -171,7 +171,7 @@ static ast_value* parser_find_typedef(parser_t *parser, const char *name, size_t
     ast_value *e;
     hash = util_hthash(parser->typedefs[0], name);
 
-    for (i = vec_size(parser->typedefs); i > upto;) {
+    for (i = parser->typedefs.size(); i > upto;) {
         --i;
         if ( (e = (ast_value*)util_htgeth(parser->typedefs[i], name, hash)) )
             return e;
@@ -1267,14 +1267,14 @@ static bool parser_close_call(parser_t *parser, shunt *sy)
      * intrinsic call and just evaluate it i.e constant fold it.
      */
     if (fold && ast_istype(fun, ast_value) && ((ast_value*)fun)->m_intrinsic) {
-        ast_expression **exprs  = nullptr;
+        std::vector<ast_expression*> exprs;
         ast_expression *foldval = nullptr;
 
+        exprs.reserve(paramcount);
         for (i = 0; i < paramcount; i++)
-            vec_push(exprs, sy->out[fid+1 + i].out);
+            exprs.push_back(sy->out[fid+1 + i].out);
 
-        if (!(foldval = parser->m_intrin.do_fold((ast_value*)fun, exprs))) {
-            vec_free(exprs);
+        if (!(foldval = parser->m_intrin.do_fold((ast_value*)fun, exprs.data()))) {
             goto fold_leave;
         }
 
@@ -1284,7 +1284,6 @@ static bool parser_close_call(parser_t *parser, shunt *sy)
          */
         sy->out[fid] = syexp(foldval->m_context, foldval);
         sy->out.erase(sy->out.end() - paramcount, sy->out.end());
-        vec_free(exprs);
 
         return true;
     }
@@ -2000,11 +1999,11 @@ static ast_expression* parse_expression(parser_t *parser, bool stopatcomma, bool
 
 static void parser_enterblock(parser_t *parser)
 {
-    vec_push(parser->variables, util_htnew(PARSER_HT_SIZE));
-    vec_push(parser->_blocklocals, vec_size(parser->_locals));
-    vec_push(parser->typedefs, util_htnew(TYPEDEF_HT_SIZE));
-    vec_push(parser->_blocktypedefs, vec_size(parser->_typedefs));
-    vec_push(parser->_block_ctx, parser_ctx(parser));
+    parser->variables.push_back(util_htnew(PARSER_HT_SIZE));
+    parser->_blocklocals.push_back(parser->_locals.size());
+    parser->typedefs.push_back(util_htnew(TYPEDEF_HT_SIZE));
+    parser->_blocktypedefs.push_back(parser->_typedefs.size());
+    parser->_block_ctx.push_back(parser_ctx(parser));
 }
 
 static bool parser_leaveblock(parser_t *parser)
@@ -2012,41 +2011,37 @@ static bool parser_leaveblock(parser_t *parser)
     bool   rv = true;
     size_t locals, typedefs;
 
-    if (vec_size(parser->variables) <= PARSER_HT_LOCALS) {
+    if (parser->variables.size() <= PARSER_HT_LOCALS) {
         parseerror(parser, "internal error: parser_leaveblock with no block");
         return false;
     }
 
-    util_htdel(vec_last(parser->variables));
+    util_htdel(parser->variables.back());
 
-    vec_pop(parser->variables);
-    if (!vec_size(parser->_blocklocals)) {
+    parser->variables.pop_back();
+    if (!parser->_blocklocals.size()) {
         parseerror(parser, "internal error: parser_leaveblock with no block (2)");
         return false;
     }
 
-    locals = vec_last(parser->_blocklocals);
-    vec_pop(parser->_blocklocals);
-    while (vec_size(parser->_locals) != locals)
-        vec_pop(parser->_locals);
+    locals = parser->_blocklocals.back();
+    parser->_blocklocals.pop_back();
+    parser->_locals.resize(locals);
 
-    typedefs = vec_last(parser->_blocktypedefs);
-    while (vec_size(parser->_typedefs) != typedefs) {
-        delete vec_last(parser->_typedefs);
-        vec_pop(parser->_typedefs);
-    }
-    util_htdel(vec_last(parser->typedefs));
-    vec_pop(parser->typedefs);
+    typedefs = parser->_blocktypedefs.back();
+    parser->_typedefs.resize(typedefs);
+    util_htdel(parser->typedefs.back());
+    parser->typedefs.pop_back();
 
-    vec_pop(parser->_block_ctx);
+    parser->_block_ctx.pop_back();
 
     return rv;
 }
 
 static void parser_addlocal(parser_t *parser, const char *name, ast_expression *e)
 {
-    vec_push(parser->_locals, e);
-    util_htset(vec_last(parser->variables), name, (void*)e);
+    parser->_locals.push_back(e);
+    util_htset(parser->variables.back(), name, (void*)e);
 }
 static void parser_addlocal(parser_t *parser, const std::string &name, ast_expression *e) {
     return parser_addlocal(parser, name.c_str(), e);
@@ -3627,9 +3622,9 @@ static bool parse_enum(parser_t *parser)
     bool        flag = false;
     bool        reverse = false;
     qcfloat_t     num = 0;
-    ast_value **values = nullptr;
     ast_value  *var = nullptr;
     ast_value  *asvalue;
+    std::vector<ast_value*> values;
 
     ast_expression *old;
 
@@ -3671,7 +3666,7 @@ static bool parse_enum(parser_t *parser)
                 break;
             }
             parseerror(parser, "expected identifier or `}`");
-            goto onerror;
+            return false;
         }
 
         old = parser_find_field(parser, parser_tokval(parser));
@@ -3680,11 +3675,11 @@ static bool parse_enum(parser_t *parser)
         if (old) {
             parseerror(parser, "value `%s` has already been declared here: %s:%i",
                        parser_tokval(parser), old->m_context.file, old->m_context.line);
-            goto onerror;
+            return false;
         }
 
         var = new ast_value(parser_ctx(parser), parser_tokval(parser), TYPE_FLOAT);
-        vec_push(values, var);
+        values.push_back(var);
         var->m_cvq             = CV_CONST;
         var->m_hasvalue        = true;
 
@@ -3694,7 +3689,7 @@ static bool parse_enum(parser_t *parser)
 
         if (!parser_next(parser)) {
             parseerror(parser, "expected `=`, `}` or comma after identifier");
-            goto onerror;
+            return false;
         }
 
         if (parser->tok == ',')
@@ -3703,12 +3698,12 @@ static bool parse_enum(parser_t *parser)
             break;
         if (parser->tok != '=') {
             parseerror(parser, "expected `=`, `}` or comma after identifier");
-            goto onerror;
+            return false;
         }
 
         if (!parser_next(parser)) {
             parseerror(parser, "expected expression after `=`");
-            goto onerror;
+            return false;
         }
 
         /* We got a value! */
@@ -3716,7 +3711,7 @@ static bool parse_enum(parser_t *parser)
         asvalue = (ast_value*)old;
         if (!ast_istype(old, ast_value) || asvalue->m_cvq != CV_CONST || !asvalue->m_hasvalue) {
             compile_error(var->m_context, "constant value or expression expected");
-            goto onerror;
+            return false;
         }
         num = (var->m_constval.vfloat = asvalue->m_constval.vfloat) + 1;
 
@@ -3724,38 +3719,33 @@ static bool parse_enum(parser_t *parser)
             break;
         if (parser->tok != ',') {
             parseerror(parser, "expected `}` or comma after expression");
-            goto onerror;
+            return false;
         }
     }
 
     /* patch them all (for reversed attribute) */
     if (reverse) {
         size_t i;
-        for (i = 0; i < vec_size(values); i++)
-            values[i]->m_constval.vfloat = vec_size(values) - i - 1;
+        for (i = 0; i < values.size(); i++)
+            values[i]->m_constval.vfloat = values.size() - i - 1;
     }
 
     if (parser->tok != '}') {
         parseerror(parser, "internal error: breaking without `}`");
-        goto onerror;
+        return false;
     }
 
     if (!parser_next(parser) || parser->tok != ';') {
         parseerror(parser, "expected semicolon after enumeration");
-        goto onerror;
+        return false;
     }
 
     if (!parser_next(parser)) {
         parseerror(parser, "parse error after enumeration");
-        goto onerror;
+        return false;
     }
 
-    vec_free(values);
     return true;
-
-onerror:
-    vec_free(values);
-    return false;
 }
 
 static bool parse_block_into(parser_t *parser, ast_block *block)
@@ -4174,7 +4164,7 @@ static bool parse_function_body(parser_t *parser, ast_value *var)
     parser->function = old;
     if (!parser_leaveblock(parser))
         retval = false;
-    if (vec_size(parser->variables) != PARSER_HT_LOCALS) {
+    if (parser->variables.size() != PARSER_HT_LOCALS) {
         parseerror(parser, "internal error: local scopes left");
         retval = false;
     }
@@ -4993,15 +4983,15 @@ static bool parse_typedef(parser_t *parser)
         return false;
     }
 
-    if ( (oldtype = parser_find_typedef(parser, typevar->m_name, vec_last(parser->_blocktypedefs))) ) {
+    if ( (oldtype = parser_find_typedef(parser, typevar->m_name, parser->_blocktypedefs.back())) ) {
         parseerror(parser, "type `%s` has already been declared here: %s:%i",
                    typevar->m_name, oldtype->m_context.file, oldtype->m_context.line);
         delete typevar;
         return false;
     }
 
-    vec_push(parser->_typedefs, typevar);
-    util_htset(vec_last(parser->typedefs), typevar->m_name.c_str(), typevar);
+    parser->_typedefs.emplace_back(typevar);
+    util_htset(parser->typedefs.back(), typevar->m_name.c_str(), typevar);
 
     if (parser->tok != ';') {
         parseerror(parser, "expected semicolon after typedef");
@@ -5368,7 +5358,7 @@ static bool parse_variable(parser_t *parser, ast_block *localblock, bool nofield
         }
         else /* it's not a global */
         {
-            old = parser_find_local(parser, var->m_name, vec_size(parser->variables)-1, &isparam);
+            old = parser_find_local(parser, var->m_name, parser->variables.size()-1, &isparam);
             if (old && !isparam) {
                 parseerror(parser, "local `%s` already declared here: %s:%i",
                            var->m_name, old->m_context.file, (int)old->m_context.line);
@@ -5495,7 +5485,7 @@ static bool parse_variable(parser_t *parser, ast_block *localblock, bool nofield
                     prefix_len = defname.length();
 
                     // Add it to the local scope
-                    util_htset(vec_last(parser->variables), var->m_name.c_str(), (void*)var);
+                    util_htset(parser->variables.back(), var->m_name.c_str(), (void*)var);
 
                     // now rename the global
                     defname.append(var->m_name);
@@ -5525,7 +5515,7 @@ static bool parse_variable(parser_t *parser, ast_block *localblock, bool nofield
                     if (isvector) {
                         defname.erase(prefix_len);
                         for (i = 0; i < 3; ++i) {
-                            util_htset(vec_last(parser->variables), me[i]->m_name.c_str(), (void*)(me[i]));
+                            util_htset(parser->variables.back(), me[i]->m_name.c_str(), (void*)(me[i]));
                             me[i]->m_name = move(defname + me[i]->m_name);
                             parser->globals.push_back(me[i]);
                         }
@@ -5763,15 +5753,15 @@ skipvar:
                 /* remove it from the current locals */
                 if (isvector) {
                     for (i = 0; i < 3; ++i) {
-                        vec_pop(parser->_locals);
+                        parser->_locals.pop_back();
                         localblock->m_collect.pop_back();
                     }
                 }
                 /* do sanity checking, this function really needs refactoring */
-                if (vec_last(parser->_locals) != var)
+                if (parser->_locals.back() != var)
                     parseerror(parser, "internal error: unexpected change in local variable handling");
                 else
-                    vec_pop(parser->_locals);
+                    parser->_locals.pop_back();
                 if (localblock->m_locals.back() != var)
                     parseerror(parser, "internal error: unexpected change in local variable handling (2)");
                 else
@@ -6025,21 +6015,72 @@ static void generate_checksum(parser_t *parser, ir_builder *ir)
     ir->m_code->crc = crc;
 }
 
+parser_t::parser_t()
+    : lex(nullptr)
+    , tok(0)
+    , ast_cleaned(false)
+    , translated(0)
+    , crc_globals(0)
+    , crc_fields(0)
+    , function(nullptr)
+    , aliases(util_htnew(PARSER_HT_SIZE))
+    , htfields(util_htnew(PARSER_HT_SIZE))
+    , htglobals(util_htnew(PARSER_HT_SIZE))
+    , assign_op(nullptr)
+    , noref(false)
+    , max_param_count(1)
+    // finish initializing the rest of the parser before initializing
+    // m_fold and m_intrin with the parser passed along
+    , m_fold()
+    , m_intrin()
+{
+    variables.push_back(htfields);
+    variables.push_back(htglobals);
+    typedefs.push_back(util_htnew(TYPEDEF_HT_SIZE));
+    _blocktypedefs.push_back(0);
+
+    lex_ctx_t empty_ctx;
+    empty_ctx.file   = "<internal>";
+    empty_ctx.line   = 0;
+    empty_ctx.column = 0;
+    nil = new ast_value(empty_ctx, "nil", TYPE_NIL);
+    nil->m_cvq = CV_CONST;
+    if (OPTS_FLAG(UNTYPED_NIL))
+        util_htset(htglobals, "nil", (void*)nil);
+
+    const_vec[0] = new ast_value(empty_ctx, "<vector.x>", TYPE_NOEXPR);
+    const_vec[1] = new ast_value(empty_ctx, "<vector.y>", TYPE_NOEXPR);
+    const_vec[2] = new ast_value(empty_ctx, "<vector.z>", TYPE_NOEXPR);
+
+    if (OPTS_OPTION_BOOL(OPTION_ADD_INFO)) {
+        reserved_version = new ast_value(empty_ctx, "reserved:version", TYPE_STRING);
+        reserved_version->m_cvq = CV_CONST;
+        reserved_version->m_hasvalue = true;
+        reserved_version->m_flags |= AST_FLAG_INCLUDE_DEF;
+        reserved_version->m_flags |= AST_FLAG_NOREF;
+        reserved_version->m_constval.vstring = util_strdup(GMQCC_FULL_VERSION_STRING);
+    } else {
+        reserved_version = nullptr;
+    }
+
+    m_fold = fold(this);
+    m_intrin = intrin(this);
+}
+
+parser_t::~parser_t()
+{
+    remove_ast();
+}
+
 parser_t *parser_create()
 {
     parser_t *parser;
-    lex_ctx_t empty_ctx;
     size_t i;
 
-    parser = (parser_t*)mem_a(sizeof(parser_t));
+    parser = new parser_t;
     if (!parser)
         return nullptr;
 
-    memset(parser, 0, sizeof(*parser));
-
-    // TODO: remove
-    new (parser) parser_t();
-
     for (i = 0; i < operator_count; ++i) {
         if (operators[i].id == opid1('=')) {
             parser->assign_op = operators+i;
@@ -6048,44 +6089,10 @@ parser_t *parser_create()
     }
     if (!parser->assign_op) {
         con_err("internal error: initializing parser: failed to find assign operator\n");
-        mem_d(parser);
+        delete parser;
         return nullptr;
     }
 
-    vec_push(parser->variables, parser->htfields  = util_htnew(PARSER_HT_SIZE));
-    vec_push(parser->variables, parser->htglobals = util_htnew(PARSER_HT_SIZE));
-    vec_push(parser->typedefs, util_htnew(TYPEDEF_HT_SIZE));
-    vec_push(parser->_blocktypedefs, 0);
-
-    parser->aliases = util_htnew(PARSER_HT_SIZE);
-
-    empty_ctx.file   = "<internal>";
-    empty_ctx.line   = 0;
-    empty_ctx.column = 0;
-    parser->nil = new ast_value(empty_ctx, "nil", TYPE_NIL);
-    parser->nil->m_cvq = CV_CONST;
-    if (OPTS_FLAG(UNTYPED_NIL))
-        util_htset(parser->htglobals, "nil", (void*)parser->nil);
-
-    parser->max_param_count = 1;
-
-    parser->const_vec[0] = new ast_value(empty_ctx, "<vector.x>", TYPE_NOEXPR);
-    parser->const_vec[1] = new ast_value(empty_ctx, "<vector.y>", TYPE_NOEXPR);
-    parser->const_vec[2] = new ast_value(empty_ctx, "<vector.z>", TYPE_NOEXPR);
-
-    if (OPTS_OPTION_BOOL(OPTION_ADD_INFO)) {
-        parser->reserved_version = new ast_value(empty_ctx, "reserved:version", TYPE_STRING);
-        parser->reserved_version->m_cvq = CV_CONST;
-        parser->reserved_version->m_hasvalue = true;
-        parser->reserved_version->m_flags |= AST_FLAG_INCLUDE_DEF;
-        parser->reserved_version->m_flags |= AST_FLAG_NOREF;
-        parser->reserved_version->m_constval.vstring = util_strdup(GMQCC_FULL_VERSION_STRING);
-    } else {
-        parser->reserved_version = nullptr;
-    }
-
-    parser->m_fold = fold(parser);
-    parser->m_intrin = intrin(parser);
     return parser;
 }
 
@@ -6141,54 +6148,42 @@ bool parser_compile_string(parser_t *parser, const char *name, const char *str,
     return parser_compile(parser);
 }
 
-static void parser_remove_ast(parser_t *parser)
+void parser_t::remove_ast()
 {
-    size_t i;
-    if (parser->ast_cleaned)
+    if (ast_cleaned)
         return;
-    parser->ast_cleaned = true;
-    for (auto &it : parser->accessors) {
+    ast_cleaned = true;
+    for (auto &it : accessors) {
         delete it->m_constval.vfunc;
         it->m_constval.vfunc = nullptr;
         delete it;
     }
-    for (auto &it : parser->functions) delete it;
-    for (auto &it : parser->globals) delete it;
-    for (auto &it : parser->fields) delete it;
+    for (auto &it : functions) delete it;
+    for (auto &it : globals) delete it;
+    for (auto &it : fields) delete it;
 
-    for (i = 0; i < vec_size(parser->variables); ++i)
-        util_htdel(parser->variables[i]);
-    vec_free(parser->variables);
-    vec_free(parser->_blocklocals);
-    vec_free(parser->_locals);
+    for (auto &it : variables) util_htdel(it);
+    variables.clear();
+    _blocklocals.clear();
+    _locals.clear();
 
-    for (i = 0; i < vec_size(parser->_typedefs); ++i)
-        delete parser->_typedefs[i];
-    vec_free(parser->_typedefs);
-    for (i = 0; i < vec_size(parser->typedefs); ++i)
-        util_htdel(parser->typedefs[i]);
-    vec_free(parser->typedefs);
-    vec_free(parser->_blocktypedefs);
+    _typedefs.clear();
+    for (auto &it : typedefs) util_htdel(it);
+    typedefs.clear();
+    _blocktypedefs.clear();
 
-    vec_free(parser->_block_ctx);
+    _block_ctx.clear();
 
-    delete parser->nil;
+    delete nil;
 
-    delete parser->const_vec[0];
-    delete parser->const_vec[1];
-    delete parser->const_vec[2];
+    delete const_vec[0];
+    delete const_vec[1];
+    delete const_vec[2];
 
-    if (parser->reserved_version)
-        delete parser->reserved_version;
+    if (reserved_version)
+        delete reserved_version;
 
-    util_htdel(parser->aliases);
-}
-
-void parser_cleanup(parser_t *parser)
-{
-    parser_remove_ast(parser);
-    parser->~parser_t();
-    mem_d(parser);
+    util_htdel(aliases);
 }
 
 static bool parser_set_coverage_func(parser_t *parser, ir_builder *ir) {
@@ -6372,7 +6367,7 @@ bool parser_finish(parser_t *parser, const char *output)
             return false;
         }
     }
-    parser_remove_ast(parser);
+    parser->remove_ast();
 
     auto fnCheckWErrors = [&retval]() {
         if (compile_Werrors) {
diff --git a/parser.h b/parser.h
index a09238d..e5361a8 100644
--- a/parser.h
+++ b/parser.h
@@ -2,7 +2,7 @@
 #define GMQCC_PARSER_HDR
 #include "gmqcc.h"
 #include "lexer.h"
-//#include "ast.h"
+#include "ast.h"
 
 #include "intrin.h"
 #include "fold.h"
@@ -12,7 +12,10 @@ struct parser_t;
 #define parser_ctx(p) ((p)->lex->tok.ctx)
 
 struct parser_t {
-    parser_t() { }
+    parser_t();
+    ~parser_t();
+
+    void remove_ast();
 
     lex_file *lex;
     int tok;
@@ -45,17 +48,17 @@ struct parser_t {
     std::vector<const char *> continues;
 
     /* A list of hashtables for each scope */
-    ht *variables;
+    std::vector<ht> variables;
     ht htfields;
     ht htglobals;
-    ht *typedefs;
+    std::vector<ht> typedefs;
 
     /* not to be used directly, we use the hash table */
-    ast_expression **_locals;
-    size_t *_blocklocals;
-    ast_value **_typedefs;
-    size_t *_blocktypedefs;
-    lex_ctx_t *_block_ctx;
+    std::vector<ast_expression*> _locals;
+    std::vector<size_t> _blocklocals;
+    std::vector<std::unique_ptr<ast_value>> _typedefs;
+    std::vector<size_t> _blocktypedefs;
+    std::vector<lex_ctx_t> _block_ctx;
 
     /* we store the '=' operator info */
     const oper_info *assign_op;