https://gcc.gnu.org/g:b5d7c044826cbb5958bbd8e7bdd5a8832444a2db

commit r17-1696-gb5d7c044826cbb5958bbd8e7bdd5a8832444a2db
Author: Olivier Hainque <[email protected]>
Date:   Thu Jun 4 12:02:23 2026 +0000

    ada: Straigthen Call_Chain and __gnat_backtrace processing
    
    Work on cross-testsuite/ta20_015__exc_traceback exposed
    that calls to GNAT.Traceback.Call_Chain, in some configurations,
    yield a first entry within Call_Chain rather than at the
    call point.
    
    This is supposed to be controlled by a careful computation
    of a number of frames to skip, passed down as the SKIP_FRAMES
    parameter to __gnat_backtrace in tracebak.c.
    
    Roughly:
    ```
     subprogramA
      calls subprogramB
       calls GNAT.TB.Call_Chain (Skip_Frames => 1)
        calls System.TB.Chain (Skip_Frames => 2)
         calls __gnat_backtrace (skip_frames => 3)
               Actual backtrace at this point is
                 1 __gnat_backtrace
                 2 System.TB.Call_Chain
                 3 GNAT.TB.Call_Chain
                 4 spB
                 5 spA
               Skip the first 3 frames -> return
                 [spB, spA]
    ```
    
    Within tracebak.c, the skip operation is influenced
    by micro details of the semantics of a BASE_SKIP macro,
    which was imprecisely documented.
    
    I first thought the fault was in the value for this macro
    in a couple of configurations, while it was entirely elswehere.
    
    There were two issues aside from the BASE_SKIP documentation
    (in)accuracy:
    
    1 GNAT.Traceback exposes a Call_Chain procedure and a Call_Chain
      function.
    
      The function has a Skip_Frames parameter, with a default value,
      and a description of how it operates wrt that parameter.
    
      The procedure doesn't accept a parameter, the spec was imprecise
      and the behavior was different from that of the function with
      the default parameter value.
    
    2 The way Skip_Frames values propagate down to __gnat_backtrace
      require the internal calls within the runtime to materialze as actual
      calls (and call frames) at run-time. This requires preventing
      inlining and sibling call optimization, which was achieved with
      dedicated compilation switches for s-traceb.adb and overlooked
      for g-traceb.adb.
    
    This patch addresses these combined points together:
    
    gcc/ada/ChangeLog:
            * tracebak.c: Improve documentation of BASE_SKIP.
            * libgnat/g-traceb.ads (Call_Chain procedure): Clarify the
            expected behavior wrt Call_Chain itself.
            * libgnat/g-traceb.adb (Call_Chain procedure): Propagate
            a Skip_Frames value to System.TB.Call_Chain accordingly.
            * libgnat/libgnat.gpr: Use the same special switches for
            g-traceb.adb as for s-traceb.adb.
            * Makefile.rtl: Likewise.
            * libgnat/s-traceb.adb: Reword top comment on the need
            to prevent optimizations. No need to specify how to do it
            here.

Diff:
---
 gcc/ada/Makefile.rtl         | 11 ++++++++---
 gcc/ada/libgnat/g-traceb.adb |  7 ++++++-
 gcc/ada/libgnat/g-traceb.ads |  9 ++++-----
 gcc/ada/libgnat/libgnat.gpr  |  8 ++++----
 gcc/ada/libgnat/s-traceb.adb |  6 ++++--
 gcc/ada/tracebak.c           | 27 +++++++++++++++------------
 6 files changed, 41 insertions(+), 27 deletions(-)

diff --git a/gcc/ada/Makefile.rtl b/gcc/ada/Makefile.rtl
index d4377098a00e..d99c28faec37 100644
--- a/gcc/ada/Makefile.rtl
+++ b/gcc/ada/Makefile.rtl
@@ -3405,15 +3405,20 @@ system.o: system.ads
          $(ADA_TARGET_PROPERTIES) \
          $(OUTPUT_OPTION)
 
-# Force no sibling call optimization on s-traceb.o so the number of stack
-# frames to be skipped when computing a call chain is not modified by
-# optimization. We don't want inlining, either.
+# Force no sibling call optimization on s-traceb.o and g-traceb.o so the
+# number of stack frames to be skipped when computing a call chain is not
+# modified by optimization. We don't want inlining, either.
 
 s-traceb.o: s-traceb.adb s-traceb.ads
        $(ADAC) -c $(ALL_ADAFLAGS) $(FORCE_DEBUG_ADAFLAGS) \
          $(NO_INLINE_ADAFLAGS) $(NO_SIBLING_ADAFLAGS) $(ADA_INCLUDES) $< \
          $(OUTPUT_OPTION)
 
+g-traceb.o: g-traceb.adb g-traceb.ads
+       $(ADAC) -c $(ALL_ADAFLAGS) $(FORCE_DEBUG_ADAFLAGS) \
+         $(NO_INLINE_ADAFLAGS) $(NO_SIBLING_ADAFLAGS) $(ADA_INCLUDES) $< \
+         $(OUTPUT_OPTION)
+
 # Compile s-tasdeb.o without optimization and with debug info so that it is
 # always possible to set conditional breakpoints on tasks.
 
diff --git a/gcc/ada/libgnat/g-traceb.adb b/gcc/ada/libgnat/g-traceb.adb
index 12939923f717..7b7d843b8700 100644
--- a/gcc/ada/libgnat/g-traceb.adb
+++ b/gcc/ada/libgnat/g-traceb.adb
@@ -31,6 +31,9 @@
 
 --  Run-time non-symbolic traceback support
 
+--  As for System.Traceback, inlining and sibling call optimizations
+--  must be prevented within this unit.
+
 with System.Traceback;
 
 package body GNAT.Traceback is
@@ -44,7 +47,9 @@ package body GNAT.Traceback is
       Len       : out Natural)
    is
    begin
-      System.Traceback.Call_Chain (Traceback, Traceback'Length, Len);
+      --  Request skipping this frame + that of our callee
+      System.Traceback.Call_Chain
+        (Traceback, Traceback'Length, Len, Skip_Frames => 2);
    end Call_Chain;
 
    function Call_Chain
diff --git a/gcc/ada/libgnat/g-traceb.ads b/gcc/ada/libgnat/g-traceb.ads
index e8d61fdfee9d..f99c2945b436 100644
--- a/gcc/ada/libgnat/g-traceb.ads
+++ b/gcc/ada/libgnat/g-traceb.ads
@@ -90,11 +90,10 @@ package GNAT.Traceback is
 
    procedure Call_Chain (Traceback : out Tracebacks_Array; Len : out Natural);
    --  Store up to Traceback'Length tracebacks corresponding to the current
-   --  call chain. The first entry stored corresponds to the deepest level
-   --  of subprogram calls. Len shows the number of traceback entries stored.
-   --  It will be equal to Traceback'Length unless the entire traceback is
-   --  shorter, in which case positions in Traceback past the Len position
-   --  are undefined on return.
+   --  call chain. The first entry is for the caller of this subprogram and
+   --  Len conveys the number of entries stored. Len will be Traceback'Length
+   --  unless the entire traceback is shorter. Positions in Traceback beyond
+   --  Len are undefined on return.
 
    function Call_Chain
      (Max_Len     : Positive;
diff --git a/gcc/ada/libgnat/libgnat.gpr b/gcc/ada/libgnat/libgnat.gpr
index fd2f225381ba..c0c0274886aa 100644
--- a/gcc/ada/libgnat/libgnat.gpr
+++ b/gcc/ada/libgnat/libgnat.gpr
@@ -26,12 +26,12 @@ library project Libgnat is
       for Switches ("C") use Libgnat_Common.C_Flags;
       for Switches ("Ada") use Libgnat_Common.Ada_Flags;
 
-      for Switches ("s-traceb.adb") use
+      for Switches ("?-traceb.adb") use
         Libgnat_Common.Ada_Flags & Libgnat_Common.Force_Debug &
         Libgnat_Common.No_Inline & Libgnat_Common.No_Sibling;
-      --  Force no sibling call optimization on s-traceb.o so the number of
-      --  stack frames to be skipped when computing a call chain is not
-      --  modified by optimization. We don.t want inlining, either.
+      --  Prevent inlining and sibling call optimization on s-traceb.o and
+      --  g-traceb.o so the number of stack frames to be skipped when computing
+      --  a call chain is not modified by optimization.
 
       for Switches ("a-except.adb") use
         Libgnat_Common.Ada_Flags & ("-O1") &
diff --git a/gcc/ada/libgnat/s-traceb.adb b/gcc/ada/libgnat/s-traceb.adb
index 78692582be3b..cbea261c5d11 100644
--- a/gcc/ada/libgnat/s-traceb.adb
+++ b/gcc/ada/libgnat/s-traceb.adb
@@ -31,8 +31,10 @@
 
 --  This is the default version of this package
 
---  Note: this unit must be compiled using -fno-optimize-sibling-calls.
---  See comment below in body of Call_Chain for details on the reason.
+--  Note: To preserve the correctness of "Skip_Frames" processing,
+--  subprogram calls here must materialize as subprogram calls (and
+--  frames) at the machine level, so inlining and sibling call
+--  optimizations must be prevented for this unit.
 
 package body System.Traceback is
 
diff --git a/gcc/ada/tracebak.c b/gcc/ada/tracebak.c
index 17c6be11f88f..2c80b2a16108 100644
--- a/gcc/ada/tracebak.c
+++ b/gcc/ada/tracebak.c
@@ -201,8 +201,8 @@ __gnat_backtrace (void **array,
    o FRAME_OFFSET, the offset, from a given frame address or frame pointer
      value, at which this layout will be found,
 
-   o FRAME_LEVEL, controls how many frames up we get at to start with,
-     from the initial frame pointer we compute by way of the GCC builtin,
+   o FRAME_LEVEL, control how many stack frames up we start from, in
+     builtin_frame_address terms.
 
      0 is most often the appropriate value. 1 may be necessary on targets
      where return addresses are saved by a function in it's caller's frame
@@ -237,19 +237,18 @@ __gnat_backtrace (void **array,
                         |                |
                         +----------------+
 
-   o BASE_SKIP,
+   o BASE_SKIP represents the initial shift incurred by the initial
+     FRAME_LEVEL + the fact that what we find on the stack are return
+     addresses. One way to see it is: starting from
 
-   Since we inherently deal with return addresses, there is an implicit shift
-   by at least one for the initial point we are able to observe in the chain.
+       ptr = (struct layout *)__builtin_frame_address(FRAME_LEVEL);
 
-   On some targets (e.g. sparc-solaris), the first return address we can
-   easily get without special code is even our caller's return address, so
-   there is a initial shift of two.
+     in __gnat_backtrace, where does ptr->return_address land?
 
-   BASE_SKIP represents this initial shift, which is the minimal "skip_frames"
-   value we support. We could add special code for the skip_frames < BASE_SKIP
-   cases. This is not done currently because there is virtually no situation
-   in which this would be useful.
+     If this is within __gnat_backtrace, BASE_SKIP should be 0. If this
+     is within the caller of __gnat_backtrace, BASE_SKIP should be 1. etc.
+
+     BASE_SKIP is the minimal "skip_frames" value we support.
 
    Finally, to account for some ABI specificities, a target may (but does
    not have to) define:
@@ -411,6 +410,8 @@ struct layout
 #define STOP_FRAME(CURRENT, TOP_STACK) \
  ((CURRENT)->next == 0 || ((long)(CURRENT)->next % __alignof__(void*)) != 0)
 
+/* builtin_frame_address(1) gets us the frame pointer of our caller,
+   where we store our own return address.  */
 #define BASE_SKIP 1
 
 /*-------------------------- SPARC Solaris or RTEMS --------------------*/
@@ -497,6 +498,8 @@ struct layout
    || (void *) ((CURRENT)->next) < (TOP_STACK)  \
    || EXTRA_STOP_CONDITION(CURRENT))
 
+/* builtin_frame_address(1) gets us the *frame* pointer of our caller, where
+   we'll find the return address from that caller to its own caller.  */
 #define BASE_SKIP (1+FRAME_LEVEL)
 
 /* On i386 architecture we check that at the call point we really have a call

Reply via email to