From: Olivier Hainque <[email protected]>
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.
Tested on x86_64-pc-linux-gnu, committed on master.
---
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 d4377098a00..d99c28faec3 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 12939923f71..7b7d843b870 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 e8d61fdfee9..f99c2945b43 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 fd2f225381b..c0c0274886a 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 78692582be3..cbea261c5d1 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 17c6be11f88..2c80b2a1610 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
--
2.53.0
