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 2EFF86F15B; Tue, 23 Aug 2022 12:14:02 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 2EFF86F15B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1661246042; bh=NS9odLfP8Ftf6hJH9w9C5Szo51XJq3gk8wCiTB+cXuY=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=DQ+clYU4lF6B3WPL22nWmAOPo8O7o6S5WazfpaiMCNZHMyu2VcopZu0/1TIC7XS/G HVuxl7q9j3NqX8WqsbmpDKktvRq9SIU2hYmy0bfUyR77P+KfWYkjdGQn5hvY6KE+Vr TF9V/uJ1NvXVJyA2D+FPbEYOQ2AlWKX2Cu/FUNZE= Received: from smtp38.i.mail.ru (smtp38.i.mail.ru [94.100.177.98]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id A126E6F15B for ; Tue, 23 Aug 2022 12:14:00 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org A126E6F15B Received: by smtp38.i.mail.ru with esmtpa (envelope-from ) id 1oQPyx-0001La-NL; Tue, 23 Aug 2022 12:14:00 +0300 Content-Type: multipart/alternative; boundary="------------Au03dlj60L0RolwhodHOQnOU" Message-ID: Date: Tue, 23 Aug 2022 12:13:59 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Content-Language: en-US To: Sergey Kaplun , Mikhail Elhimov Cc: tarantool-patches@dev.tarantool.org References: <2cb4fea69537ffd196d535a7d3d47c0511b0ecac.1658531255.git.m.elhimov@vk.team> In-Reply-To: X-Mailru-Src: smtp X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD999D8F08CF16C6CA778D25EAFB9B704D71619BFC4E0A6BA3500894C459B0CD1B9D0AAAA2EE7A5E0A22121CE47B21F23597E8E58CE8D4F44D05709D77201D190DA X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE771540F9ECFC94C4BEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006379FED03E15F715FAA8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D872EC1814524D9B0BF8FD2F41813BDCD46F9789CCF6C18C3F8528715B7D10C86878DA827A17800CE7E4DF6D1C10F22F599FA2833FD35BB23D9E625A9149C048EE33AC447995A7AD18C26CFBAC0749D213D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8B60CDF180582EB8FBA471835C12D1D977C4224003CC836476EB9C4185024447017B076A6E789B0E975F5C1EE8F4F765FCEF4E0C265C76411D3AA81AA40904B5D9CF19DD082D7633A078D18283394535A93AA81AA40904B5D98AA50765F79006373D42D4F566CB30C1D81D268191BDAD3D698AB9A7B718F8C4D1B931868CE1C5781A620F70A64A45A98AA50765F79006372E808ACE2090B5E1725E5C173C3A84C3C5EA940A35A165FF2DBA43225CD8A89FB2946BF27A8A49145E1C53F199C2BB95B5C8C57E37DE458BEDA766A37F9254B7 X-B7AD71C0: 1B70FBA5C9BEEE72C9761FC34675ADEB871C96603B655635EE9D5CB6078CC77C7724A3493910BA4B135824B3FEAB4495 X-C1DE0DAB: 9604B64F49C60606AD91A466A1DEF99B296C473AB1E142185AC9E3593CE4B31AB1881A6453793CE9274300E5CE05BD4401A9E91200F654B0CBAED148C0C85B12D9A1D0B5FC830B80FF58DF2F9647C10EE62F87C0B7F08F9E9C2B6934AE262D3EE7EAB7254005DCED8DA55E71E02F9FC08E8E86DC7131B365E7726E8460B7C23C X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D348BE83DFD8AFB1CAC23F2B39735688D58B18B82AC43583F7BD781EEA26673EA4FBCEEF2E34AC718F61D7E09C32AA3244C4EC8AE01CBB41B347C281DB11BE39E0324AF4FAF06DA24FDFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojzEOgLjt8B1pZ+KK1npuqiQ== X-Mailru-Sender: 391A9C92F0A95CC617713B38CE9D64BB61D7EC7979509F0DD659BE66C0138041332C412ACEE59CE6893A38EEAA9F8901AD583E2F744EC1B103B957AC4C054F678DF4F02B79026D381B698827755E8382E5AA6234683BE64C3DDE9B364B0DF289AE208404248635DF X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH 2/2] gdb: refactor iteration over frames while dumping stack 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: Mikhail Elhimov via Tarantool-patches Reply-To: Mikhail Elhimov Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" This is a multi-part message in MIME format. --------------Au03dlj60L0RolwhodHOQnOU Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Hi, Sergey! Thanks for the review! On 20.08.2022 09:28, Sergey Kaplun wrote: > Hi, Mikhail! > > Thanks for the patch! > > It's generally LGTM, except a bunch of nits and typos. > BTW, I like your iterator approach: the code looks so much better now! > > On 23.07.22, Mikhail Elhimov via Tarantool-patches wrote: >> Changes: >> - Introduce generator to iterate over frames in a Python way >> - Dump framelink of the bottom frame in a common way and identify it as >> a dummy framelink only inside of common dump function > Typo: s/of common/of the common/ Fixed >> - Framelink always refers to the slot right before frame's base > Don't get this change bullet description. Looks like this is just the > way you implement frame iterator, isn't it? Fixed Yes, this bullet described both "what" and "how" of the change. I turned this bullet into "Iterate over frames in a Python way (with iterator)" >> - Introduce constant LJ_FRAMELINK_NUM_SLOTS to make slot arithmetic >> more obvious. Previous using of boolean LJ_RT2 in arithmetic looked > Typo: s/LJ_RT2/LJ_FR2 Fixed >> like a tricky way, both to read and maintain (however places where > Side note: As for me it looks like obvious and natural :). But maybe > this is the ill effect from LuaJIT sources. I leave comments in places > where I suggest to use LJ_FR2 instead LJ_FRAMELINK_NUM_SLOTS. My point is: LJ_FR2 -- for boolean context, LJ_FRAMELINK_NUM_SLOTS -- for formulas Let me share my considerations. I see LJ_FR2 as a boolean entity that means either 1 or 2 slots are used to store "framelink" and since it is boolean, the only natural usage for it from my point of view is boolean context (i.e. if ... else ...). I understand that using boolean in formulas reduces branching and helps to improve performance, but here it doesn't seem to be necessary. For me operating with "number of slots" is easier than with "number of extra slots over 1" (which LJ_FR2 turns into in formulas). The latter is ok when it's standalone, but when it appears in formula I feel that I need to perform some additional calculation in my mind while reading code, that's why I find it less-readable and introduced LJ_FRAMELINK_NUM_SLOTS and that's why I would keep using LJ_FRAMELINK_NUM_SLOTS in formulas, and wouldn't turn back LJ_FR2 even when `LJ_FRAMELINK_NUM_SLOTS - 1` occurs An example: red = 5 + 2 * LJ_FR2 which looks a bit magic, turns into red = 3 + 2 * LJ_FRAMELINK_NUM_SLOTS which produce the same results, but literally says that we reserve 3 data slots for 2 frames (I just realized that I forgot to ask the confirmation regarding my interpretation of the number of slots reserved and left the original formula intact, so if it's correct I'd adjust it) These considerations may be changed when I dive deeper into luajit, but for now they are )) >> LJ_RT2 approach had been originally replicated from the source code > Typo: s/LJ_RT2/LJ_FR2 Fixed >> are left intact, because it is expected that maintenance efforts for >> them will be minimal) > General: Missed dot at the end of the sentences. Fixed >> --- >> src/luajit-gdb.py | 101 ++++++++++++++++++++++------------------------ >> 1 file changed, 49 insertions(+), 52 deletions(-) >> >> diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py >> index 1e9a96fb..d7d34c70 100644 >> --- a/src/luajit-gdb.py >> +++ b/src/luajit-gdb.py >> @@ -158,6 +158,7 @@ LJ_64 = None > > >> @@ -299,6 +300,21 @@ gclen = { >> 'mmudata': gcringlen, >> } >> >> +def get_framelink_sentinel(L): >> + stack = mref('TValue *', L['stack']) >> + return stack + LJ_FRAMELINK_NUM_SLOTS - 1 > `LJ_FRAMELINK_NUM_SLOTS - 1 == LJ_FR2` so we can replace it like the > following: > | return stack + LJ_FR2 > >> + >> +# Generator that implements frame iterator > Typo: s/Generator/The generator/ > Typo: s/iterator/iterator./ Fixed >> +# every frame is represented as a tuple of framelink and frametop > Typo: s/every/Every/ > Typo: s/frametop/frametop./ Fixed > Minor: I suggest to specify that this is yielded to the caller tuple. > Feel free to ignore. Sorry, I didn't catch the suggestion. Would you, please, explain? >> +def frames(L): >> + frametop = L['top'] >> + framelink = L['base'] - 1 >> + framelink_sentinel = get_framelink_sentinel(L) >> + while framelink is not None: > Minor: `is not None` is excess, we can just omit it. I'd prefer to follow the recommendation from PEP8 ========= Also, beware of writing if x when you really mean if x is not None – e.g. when testing whether a variable or argument that defaults to None was set to some other value. The other value might have a type (such as a container) that could be false in a boolean context! ========= However while checking this code I found 2-step loop exit: first, inside the loop we check that sentinel is achieved, assign `None` and catch it immediately at checking loop condition. I adjusted loop as follows: |while True:|| ||        yield framelink, frametop|| ||        frametop = framelink - LJ_FRAMELINK_NUM_SLOTS|| ||        if framelink <= framelink_sentinel:|| ||            break|| ||        framelink = frame_prev(framelink)|| | so finally `is not None` was removed, but for another reason ) >> + yield framelink, frametop >> + frametop = framelink - LJ_FRAMELINK_NUM_SLOTS >> + framelink = frame_prev(framelink) if framelink > framelink_sentinel else None >> + >> # Dumpers {{{ >> >> def dump_lj_tnil(tv): >> @@ -397,32 +413,38 @@ dumpers = { >> def dump_tvalue(tvalue): >> return dumpers.get(typenames(itypemap(tvalue)), dump_lj_invalid)(tvalue) >> >> +def dump_framelink_slot_address(fr): >> + return '{}:{}'.format(fr - 1, fr) if LJ_FR2 else '{}'.format(fr) + PADDING >> + >> +def dump_framelink_func(fr): >> + return dump_lj_tfunc(fr - 1 if LJ_FR2 else fr) > We can replace it with the following: > | return dump_lj_tfunc(fr - LJ_FR2) Please, see my consideration above >> + >> def dump_framelink(L, fr): > > >> ) >> >> -def dump_stack_slot(L, slot, base=None, top=None, eol='\n'): >> +def dump_stack_slot(L, slot, base=None, top=None): > > >> >> def dump_stack(L, base=None, top=None): > > >> + for framelink, frametop in frames(L): >> + # dump all data slots in the (framelink, top) interval > Typo: s/dump/Dump/ > Typo: s/interval/interval./ Fixed >> + dump.extend([ >> + dump_stack_slot(L, framelink + offset, base, top) >> + for offset in range(frametop - framelink, 0, -1) >> + ]) >> + # dump frame slot (2 slots in case of GC64) > Typo: s/dump/Dump/ > Missed dot at the end of the sentence. Fixed >> + dump.append( dump_framelink(L, framelink) ) >> + >> + return '\n'.join(dump) >> >> def dump_gc(g): >> gc = g['gc'] >> @@ -717,7 +713,7 @@ The command requires no args and dumps current GC stats: > > >> @@ -759,6 +755,7 @@ def init(commands): >> LJ_64 = str(gdb.parse_and_eval('IRT_PTR')) == 'IRT_P64' >> LJ_FR2 = LJ_GC64 = str(gdb.parse_and_eval('IRT_PGC')) == 'IRT_P64' >> LJ_DUALNUM = gdb.lookup_global_symbol('lj_lib_checknumber') is not None >> + LJ_FRAMELINK_NUM_SLOTS = 2 if LJ_FR2 else 1 > As far as LJ_FR2 is defined before, we can initialize this variable like > the following: > | LJ_FRAMELINK_NUM_SLOTS = LJ_FR2 + 1 Please, see my consideration above >> except: >> gdb.write('luajit-gdb.py failed to load: ' >> 'no debugging symbols found for libluajit\n') >> -- >> 2.34.1 >> The corrected commit message: =================================================== gdb: refactor iteration over frames while dumping stack Changes: - Iterate over frames in a Python way (with iterator) - Dump framelink of the bottom frame in a common way and identify it as a dummy framelink only inside of the common dump function - Framelink always refers to the slot right before frame's base - Introduce constant LJ_FRAMELINK_NUM_SLOTS to make slot arithmetic more obvious. Previous using of boolean LJ_FR2 in arithmetic looked like a tricky way, both to read and maintain (however places where LJ_FR2 approach had been originally replicated from the source code are left intact, because it is expected that maintenance efforts for them will be minimal). =================================================== diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py index d7d34c70..2c11057b 100644 --- a/src/luajit-gdb.py +++ b/src/luajit-gdb.py @@ -304,16 +304,18 @@ def get_framelink_sentinel(L): stack = mref('TValue *', L['stack']) return stack + LJ_FRAMELINK_NUM_SLOTS - 1 -# Generator that implements frame iterator -# every frame is represented as a tuple of framelink and frametop +# The generator that implements frame iterator. +# Every frame is represented as a tuple of framelink and frametop. def frames(L): frametop = L['top'] framelink = L['base'] - 1 framelink_sentinel = get_framelink_sentinel(L) - while framelink is not None: + while True: yield framelink, frametop frametop = framelink - LJ_FRAMELINK_NUM_SLOTS - framelink = frame_prev(framelink) if framelink > framelink_sentinel else None + if framelink <= framelink_sentinel: + break + framelink = frame_prev(framelink) # Dumpers {{{ @@ -477,12 +479,12 @@ def dump_stack(L, base=None, top=None): ]) for framelink, frametop in frames(L): - # dump all data slots in the (framelink, top) interval + # Dump all data slots in the (framelink, top) interval. dump.extend([ dump_stack_slot(L, framelink + offset, base, top) for offset in range(frametop - framelink, 0, -1) ]) - # dump frame slot (2 slots in case of GC64) + # Dump frame slot (2 slots in case of GC64). dump.append( dump_framelink(L, framelink) ) return '\n'.join(dump) =================================================== -- Best regards, Mikhail Elhimov --------------Au03dlj60L0RolwhodHOQnOU Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit

Hi, Sergey!

Thanks for the review!

On 20.08.2022 09:28, Sergey Kaplun wrote:
Hi, Mikhail!

Thanks for the patch!

It's generally LGTM, except a bunch of nits and typos.
BTW, I like your iterator approach: the code looks so much better now!

On 23.07.22, Mikhail Elhimov via Tarantool-patches wrote:

Changes:
- Introduce generator to iterate over frames in a Python way
- Dump framelink of the bottom frame in a common way and identify it as
  a dummy framelink only inside of common dump function

Typo: s/of common/of the common/
Fixed
- Framelink always refers to the slot right before frame's base

Don't get this change bullet description. Looks like this is just the
way you implement frame iterator, isn't it?

Fixed

Yes, this bullet described both "what" and "how" of the change. I turned this bullet into "Iterate over frames in a Python way (with iterator)"

- Introduce constant LJ_FRAMELINK_NUM_SLOTS to make slot arithmetic
  more obvious. Previous using of boolean LJ_RT2 in arithmetic looked

Typo: s/LJ_RT2/LJ_FR2
Fixed 
  like a tricky way, both to read and maintain (however places where

Side note: As for me it looks like obvious and natural :). But maybe
this is the ill effect from LuaJIT sources. I leave comments in places
where I suggest to use LJ_FR2 instead LJ_FRAMELINK_NUM_SLOTS.

My point is: LJ_FR2 -- for boolean context, LJ_FRAMELINK_NUM_SLOTS -- for formulas

Let me share my considerations.

I see LJ_FR2 as a boolean entity that means either 1 or 2 slots are used to store "framelink" and since it is boolean, the only natural usage for it from my point of view is boolean context (i.e. if ... else ...). I understand that using boolean in formulas reduces branching and helps to improve performance, but here it doesn't seem to be necessary. For me operating with "number of slots" is easier than with "number of extra slots over 1" (which LJ_FR2 turns into in formulas). The latter is ok when it's standalone, but when it appears in formula I feel that I need to perform some additional calculation in my mind while reading code, that's why I find it less-readable and introduced LJ_FRAMELINK_NUM_SLOTS and that's why I would keep using LJ_FRAMELINK_NUM_SLOTS in formulas, and wouldn't turn back LJ_FR2 even when `LJ_FRAMELINK_NUM_SLOTS - 1` occurs

An example:

red = 5 + 2 * LJ_FR2

which looks a bit magic, turns into

red = 3 + 2 * LJ_FRAMELINK_NUM_SLOTS

which produce the same results, but literally says that we reserve 3 data slots for 2 frames (I just realized that I forgot to ask the confirmation regarding my interpretation of the number of slots reserved and left the original formula intact, so if it's correct I'd adjust it)

These considerations may be changed when I dive deeper into luajit, but for now they are )) 

  LJ_RT2 approach had been originally replicated from the source code

Typo: s/LJ_RT2/LJ_FR2
Fixed 
  are left intact, because it is expected that maintenance efforts for
  them will be minimal)

General: Missed dot at the end of the sentences.
Fixed 
---
 src/luajit-gdb.py | 101 ++++++++++++++++++++++------------------------
 1 file changed, 49 insertions(+), 52 deletions(-)

diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py
index 1e9a96fb..d7d34c70 100644
--- a/src/luajit-gdb.py
+++ b/src/luajit-gdb.py
@@ -158,6 +158,7 @@ LJ_64 = None

<snipped>


@@ -299,6 +300,21 @@ gclen = {
     'mmudata': gcringlen,
 }
 
+def get_framelink_sentinel(L):
+    stack = mref('TValue *', L['stack'])
+    return stack + LJ_FRAMELINK_NUM_SLOTS - 1

`LJ_FRAMELINK_NUM_SLOTS - 1 == LJ_FR2` so we can replace it like the
following:
| return stack + LJ_FR2


+
+# Generator that implements frame iterator

Typo: s/Generator/The generator/
Typo: s/iterator/iterator./
Fixed 
+# every frame is represented as a tuple of framelink and frametop

Typo: s/every/Every/
Typo: s/frametop/frametop./
Fixed 
Minor: I suggest to specify that this is yielded to the caller tuple.
Feel free to ignore.
Sorry, I didn't catch the suggestion. Would you, please, explain? 
+def frames(L):
+    frametop = L['top']
+    framelink = L['base'] - 1
+    framelink_sentinel = get_framelink_sentinel(L)
+    while framelink is not None:

Minor: `is not None` is excess, we can just omit it.

I'd prefer to follow the recommendation from PEP8

=========

Also, beware of writing if x when you really mean if x is not None – e.g. when testing whether a variable or argument that defaults to None was set to some other value. The other value might have a type (such as a container) that could be false in a boolean context!

=========

However while checking this code I found 2-step loop exit: first, inside the loop we check that sentinel is achieved, assign `None` and catch it immediately at checking loop condition. I adjusted loop as follows:

while True:
        yield framelink, frametop
        frametop = framelink - LJ_FRAMELINK_NUM_SLOTS
        if framelink <= framelink_sentinel:
            break
        framelink = frame_prev(framelink)

so finally `is not None` was removed, but for another reason ) 

+        yield framelink, frametop
+        frametop = framelink - LJ_FRAMELINK_NUM_SLOTS
+        framelink = frame_prev(framelink) if framelink > framelink_sentinel else None
+
 # Dumpers {{{
 
 def dump_lj_tnil(tv):
@@ -397,32 +413,38 @@ dumpers = {
 def dump_tvalue(tvalue):
     return dumpers.get(typenames(itypemap(tvalue)), dump_lj_invalid)(tvalue)
 
+def dump_framelink_slot_address(fr):
+    return '{}:{}'.format(fr - 1, fr) if LJ_FR2 else '{}'.format(fr) + PADDING
+
+def dump_framelink_func(fr):
+    return dump_lj_tfunc(fr - 1 if LJ_FR2 else fr)

We can replace it with the following:
| return dump_lj_tfunc(fr - LJ_FR2)
Please, see my consideration above 
+
 def dump_framelink(L, fr):

<snipped>


     )
 
-def dump_stack_slot(L, slot, base=None, top=None, eol='\n'):
+def dump_stack_slot(L, slot, base=None, top=None):

<snipped>


 
 def dump_stack(L, base=None, top=None):

<snipped>


+    for framelink, frametop in frames(L):
+        # dump all data slots in the (framelink, top) interval

Typo: s/dump/Dump/
Typo: s/interval/interval./
Fixed 
+        dump.extend([
+            dump_stack_slot(L, framelink + offset, base, top)
+                for offset in range(frametop - framelink, 0, -1)
+        ])
+        # dump frame slot (2 slots in case of GC64)

Typo: s/dump/Dump/
Missed dot at the end of the sentence.
Fixed 
+        dump.append( dump_framelink(L, framelink) )
+
+    return '\n'.join(dump)
 
 def dump_gc(g):
     gc = g['gc']
@@ -717,7 +713,7 @@ The command requires no args and dumps current GC stats:

<snipped>


@@ -759,6 +755,7 @@ def init(commands):
         LJ_64 = str(gdb.parse_and_eval('IRT_PTR')) == 'IRT_P64'
         LJ_FR2 = LJ_GC64 = str(gdb.parse_and_eval('IRT_PGC')) == 'IRT_P64'
         LJ_DUALNUM = gdb.lookup_global_symbol('lj_lib_checknumber') is not None
+        LJ_FRAMELINK_NUM_SLOTS = 2 if LJ_FR2 else 1

As far as LJ_FR2 is defined before, we can initialize this variable like
the following:
| LJ_FRAMELINK_NUM_SLOTS = LJ_FR2 + 1
Please, see my consideration above 
     except:
         gdb.write('luajit-gdb.py failed to load: '
                   'no debugging symbols found for libluajit\n')
-- 
2.34.1


The corrected commit message:
===================================================
gdb: refactor iteration over frames while dumping stack

Changes:
- Iterate over frames in a Python way (with iterator)
- Dump framelink of the bottom frame in a common way and identify it as
  a dummy framelink only inside of the common dump function
- Framelink always refers to the slot right before frame's base
- Introduce constant LJ_FRAMELINK_NUM_SLOTS to make slot arithmetic
  more obvious. Previous using of boolean LJ_FR2 in arithmetic looked
  like a tricky way, both to read and maintain (however places where
  LJ_FR2 approach had been originally replicated from the source code
  are left intact, because it is expected that maintenance efforts for
  them will be minimal).

===================================================
diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py
index d7d34c70..2c11057b 100644
--- a/src/luajit-gdb.py
+++ b/src/luajit-gdb.py
@@ -304,16 +304,18 @@ def get_framelink_sentinel(L):
     stack = mref('TValue *', L['stack'])
     return stack + LJ_FRAMELINK_NUM_SLOTS - 1
 
-# Generator that implements frame iterator
-# every frame is represented as a tuple of framelink and frametop
+# The generator that implements frame iterator.
+# Every frame is represented as a tuple of framelink and frametop.
 def frames(L):
     frametop = L['top']
     framelink = L['base'] - 1
     framelink_sentinel = get_framelink_sentinel(L)
-    while framelink is not None:
+    while True:
         yield framelink, frametop
         frametop = framelink - LJ_FRAMELINK_NUM_SLOTS
-        framelink = frame_prev(framelink) if framelink > framelink_sentinel else None
+        if framelink <= framelink_sentinel:
+            break
+        framelink = frame_prev(framelink)
 
 # Dumpers {{{
 
@@ -477,12 +479,12 @@ def dump_stack(L, base=None, top=None):
     ])
 
     for framelink, frametop in frames(L):
-        # dump all data slots in the (framelink, top) interval
+        # Dump all data slots in the (framelink, top) interval.
         dump.extend([
             dump_stack_slot(L, framelink + offset, base, top)
                 for offset in range(frametop - framelink, 0, -1)
         ])
-        # dump frame slot (2 slots in case of GC64)
+        # Dump frame slot (2 slots in case of GC64).
         dump.append( dump_framelink(L, framelink) )
 
     return '\n'.join(dump)
===================================================


-- 
Best regards,
Mikhail Elhimov
--------------Au03dlj60L0RolwhodHOQnOU--