From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id D5860A022E8; Thu, 1 Feb 2024 14:25:58 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org D5860A022E8 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1706786758; bh=VUY5+3KKlVIcb/aaBxqcD8JFzh88ElglimGLSB+GOFU=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=dKKEQc4psoP9yhyxiPKP4Ac/wdmVAk1KnCnVEk+H9I95LIMLSSjPU4uBpbd57AesT smaInBAC3GNeP8ik1T7QoRdAtHYWP4rhaD/CE6Ek2kuQuiksOk69vpHP0o0YrzI6Q/ Ynqf7EGT2mcFh4W1UHtAMRTYbRMP/Fd+RP88cBlY= Received: from mail-lj1-f175.google.com (mail-lj1-f175.google.com [209.85.208.175]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id C12C5A022EA for ; Thu, 1 Feb 2024 14:25:03 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org C12C5A022EA Received: by mail-lj1-f175.google.com with SMTP id 38308e7fff4ca-2cf33b909e8so18486661fa.0 for ; Thu, 01 Feb 2024 03:25:03 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706786703; x=1707391503; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=SavgSW2QHqF5nuMLCF3dFxn8Na7vTraT2+I86v6U+pw=; b=gdAaBx2nndWXyKK+nHqLIE9+G3gRk9voieYtIwukNUKcUGUneTzSg2davZ11qGzqAB EIruIipxESsyMHyk+JddeWGJ4m5HhooGRAouQRTIg/A57jXsz9aeimufnB0Kdhzr/Mbz Ryh1FYxtSJ/NoccpE2cbFr0DPKKwjwEli25MXfLHMCA3bTsIkgVeFBLD1HwkKod1DyOS rHOba9EtQTkTIIswHVQsA5r6GvmJy5Q9XVimlSLigzazRox/fmf2tcKT4gieglZVVzpf FzI3TpZqR7705Bwm4QdGaHaySaMzvBztcH3XmGk1vrPezSrZfe14xJf3xzRpdWWbEEoT fVfA== X-Gm-Message-State: AOJu0YwB6auodZ/MgCThqXARPeK1XlwmxNxpI5Gecdk2Jr6EZUgOt0V7 aJY2v7SHQO8enG2N2s1m3yJ1/d/Vd06ZtSAA/P//DVWuhdi1v6yds9LFF4wv X-Google-Smtp-Source: AGHT+IElWfghzh1EVjOPLTCB0S5VePM6aX95lj96nKr9cA5aFAHhkei/h7U7H9e8cxRPGLZZPa2DHw== X-Received: by 2002:a2e:925a:0:b0:2cf:1764:891d with SMTP id v26-20020a2e925a000000b002cf1764891dmr2593441ljg.1.1706786702848; Thu, 01 Feb 2024 03:25:02 -0800 (PST) X-Forwarded-Encrypted: i=0; AJvYcCUxZ1jjS2dEB74T5KU8+RTgMM+BaZ5SB9BQbeigjka8lYNfPRmPlCRAJcOz+FeWEzIsPne7/5CyBLI5H/fm8DEIsdGmgK0CxjyorBmddAn09RNjQNAw6VfUjRcYLA== Received: from localhost.localdomain (95-24-0-113.broadband.corbina.ru. [95.24.0.113]) by smtp.gmail.com with ESMTPSA id z9-20020a2e3509000000b002cdf37ee19dsm2372647ljz.7.2024.02.01.03.25.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 01 Feb 2024 03:25:02 -0800 (PST) X-Google-Original-From: Maxim Kokryashkin To: tarantool-patches@dev.tarantool.org, skaplun@tarantool.org, sergeyb@tarantool.org Date: Thu, 1 Feb 2024 14:24:47 +0300 Message-ID: <7b13a099e55f970ac99463afaf80c32f421e1cb7.1706786622.git.m.kokryashkin@tarantool.org> X-Mailer: git-send-email 2.43.0 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH luajit v4 2/3] Cleanup stack overflow handling. X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Maxim Kokryashkin via Tarantool-patches Reply-To: Maxim Kokryashkin Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" From: Mike Pall Reported by Peter Cawley. (cherry-picked from commit d2f6c55b05c716e5dbb479b7e684abaee7cf6e12) After the previous patch, it is possible to trigger the `stack overflow` error prematurely. Consider the following situation: there are already 33000 slots allocated on a Lua stack, and then there are 30 additional slots needed. In this case, the actual allocated amount would be twice the already allocated size, shrunk to the `LJ_STACK_MAXEX` size, which would lead to the stack overflow error, despite the fact there is plenty of unused space. This patch completely reworks the logic of error handling during stack growth to address the issue. Another important thing to notice is that the `LJ_ERR_STKOV` is thrown only if the `L->status` is `LUA_OK` and that status is set to `LUA_ERRRUN` just before throwing the error. The status is set to `LUA_ERRRUN` to avoid the second stack overflow during the `err_msgv` execution. Maxim Kokryashkin: * added the description and the test for the problem Part of tarantool/tarantool#9145 --- src/lj_state.c | 15 +++-- .../lj-962-premature-stack-overflow.test.c | 63 +++++++++++++++++++ 2 files changed, 74 insertions(+), 4 deletions(-) create mode 100644 test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c diff --git a/src/lj_state.c b/src/lj_state.c index 76153bad..d8a5134c 100644 --- a/src/lj_state.c +++ b/src/lj_state.c @@ -121,8 +121,17 @@ void lj_state_shrinkstack(lua_State *L, MSize used) void LJ_FASTCALL lj_state_growstack(lua_State *L, MSize need) { MSize n; - if (L->stacksize > LJ_STACK_MAXEX) /* Overflow while handling overflow? */ - lj_err_throw(L, LUA_ERRERR); + if (L->stacksize >= LJ_STACK_MAXEX) { + /* 4. Throw 'error in error handling' when we are _over_ the limit. */ + if (L->stacksize > LJ_STACK_MAXEX) + lj_err_throw(L, LUA_ERRERR); /* Does not invoke an error handler. */ + /* 1. We are _at_ the limit after the last growth. */ + if (!L->status) { /* 2. Throw 'stack overflow'. */ + L->status = LUA_ERRRUN; /* Prevent ending here again for pushed msg. */ + lj_err_msg(L, LJ_ERR_STKOV); /* May invoke an error handler. */ + } + /* 3. Add space (over the limit) for pushed message and error handler. */ + } n = L->stacksize + need; if (n > LJ_STACK_MAX) { n += 2*LUA_MINSTACK; @@ -132,8 +141,6 @@ void LJ_FASTCALL lj_state_growstack(lua_State *L, MSize need) n = LJ_STACK_MAX; } resizestack(L, n); - if (L->stacksize >= LJ_STACK_MAXEX) - lj_err_msg(L, LJ_ERR_STKOV); } void LJ_FASTCALL lj_state_growstack1(lua_State *L) diff --git a/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c b/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c new file mode 100644 index 00000000..12cb9004 --- /dev/null +++ b/test/tarantool-c-tests/lj-962-premature-stack-overflow.test.c @@ -0,0 +1,63 @@ +#include "lua.h" +#include "lauxlib.h" + +#include "test.h" +#include "utils.h" + +/* + * XXX: The "lj_obj.h" header is included to calculate the + * number of stack slots used from the bottom of the stack. + */ +#include "lj_obj.h" + +static int cur_slots = -1; + +static int fill_stack(lua_State *L) +{ + cur_slots = L->base - tvref(L->stack); + + while(lua_gettop(L) < LUAI_MAXSTACK) { + cur_slots += 1; + lua_pushinteger(L, 42); + } + + return 0; +} + +static int premature_stackoverflow(void *test_state) +{ + lua_State *L = test_state; + lua_cpcall(L, fill_stack, NULL); + assert_true(cur_slots == LUAI_MAXSTACK - 1); + return TEST_EXIT_SUCCESS; +} + +/* + * XXX: This test should fail neither before the patch + * nor after it. + */ +static int stackoverflow_during_stackoverflow(void *test_state) +{ + lua_State *L = test_state; + /* + * XXX: `fill_stack` acts here as its own error handler, + * causing the second stack overflow. + */ + lua_pushcfunction(L, fill_stack); + lua_pushcfunction(L, fill_stack); + int status = lua_pcall(L, 0, 0, -2); + assert_true(status == LUA_ERRERR); + return TEST_EXIT_SUCCESS; +} + +int main(void) +{ + lua_State *L = utils_lua_init(); + const struct test_unit tgroup[] = { + test_unit_def(premature_stackoverflow), + test_unit_def(stackoverflow_during_stackoverflow), + }; + const int test_result = test_run_group(tgroup, L); + utils_lua_close(L); + return test_result; +} -- 2.43.0