dtrace/-DDEBUGGING builds fail on multiple platforms #15182
Comments
From @tonycozRecently Solaris builds with both -DDEBUGGING and -Dusedtrace have started failing /opt/solarisstudio12.3/bin/cc -o libperl.so -G -L/usr/lib -L/usr/ccs/lib -L/opt/solarisstudio12.3/prod/lib -L/lib -L/usr/gnu/lib op.o perl.o gv.o toke.o perly.o pad.o regcomp.o dump.o util.o mg.o reentr.o mro_core.o keywords.o hv.o av.o run.o pp_hot.o sv.o pp.o scope.o pp_ctl.o pp_sys.o doop.o doio.o regexec.o utf8.o taint.o deb.o universal.o globals.o perlio.o perlapi.o numeric.o mathoms.o locale.o pp_pack.o pp_sort.o caretx.o dquote.o time64.o perldtrace.o DynaLoader.o -lpthread -lsocket -lnsl -ldl -lm -lc |
From @tonycozOn Sun Feb 14 18:26:50 2016, tonyc wrote:
This seems to have been introduced by: commit a73d881 convert CX_PUSHSUB/POPSUB to inline fns Inline Patchdiff --git a/cop.h b/cop.h
index 3a7afb5..d7ae2d6 100644
--- a/cop.h
+++ b/cop.h
@@ -595,20 +595,6 @@ struct block_format {
* The context frame holds a reference to the CV so that it can't be
* freed while we're executing it */
I don't see why this only happens in -DDEBUGGING builds. Adding the fairly obvious change to Makefile.SH: Inline Patchdiff --git a/Makefile.SH b/Makefile.SH
index 2278f9f..4377d10 100755
--- a/Makefile.SH
+++ b/Makefile.SH
@@ -841,8 +841,8 @@ mydtrace.h: $(DTRACE_H)
case "$dtrace_o" in
?*)
$spitshell >>$Makefile <<'!NO!SUBS!'
-$(DTRACE_O): perldtrace.d $(ndt_obj)
- $(DTRACE) -G -s perldtrace.d -o $(DTRACE_O) $(ndt_obj)
+$(DTRACE_O): perldtrace.d $(ndt_obj) perlmain$(OBJ_EXT)
+ $(DTRACE) -G -s perldtrace.d -o $(DTRACE_O) $(ndt_obj) perlmain$(OBJ_EXT
$(MINIDTRACE_O): perldtrace.d $(minindt_obj) perlmini$(OBJ_EXT)
$(DTRACE) -G -s perldtrace.d -o $(MINIDTRACE_O) $(minindt_obj) perlmini$
LD_LIBRARY_PATH=/home/tony/dev/perl/git/perl ./perl -Ilib -f pod/buildtoc -q so I suspect the dtrace calls will need to hoisted out of the inline functions into wrapper macros and we need to *not* apply the Makefile.SH change above. This is similar to inline.h problems we've had in the past, where symbols referenced by unused inline functions have still caused references to those symbols. Tony |
From @tonycozOn Sun Feb 14 21:00:17 2016, tonyc wrote:
Possibly because the assertions make the code large enough that the compiler doesn't want to inline them anymore. Tony |
From @iabynOn Sun, Feb 14, 2016 at 09:00:17PM -0800, Tony Cook via RT wrote:
Well I don't understand why the static inlining is tripping up Solaris, Can you let me know if it fixes the Solaris issue? It appears that t/run/dtrace.t doesn't play well on Linux, since the If I've read things right, t/run/dtrace.t would have to change calling -- |
|
The RT System itself - Status changed from 'new' to 'open' |
From @iabynOn Mon, Feb 15, 2016 at 03:33:38PM +0000, Dave Mitchell wrote:
and now as smoke-me/davem/dtrace2, with added makedef.pl goodness to keep -- |
From @tonycozOn Mon, Feb 15, 2016 at 04:45:35PM +0000, Dave Mitchell wrote:
Still a problem: Undefined first referenced Caused by PERL_SUB_ENTRY_ENABLED(). Tony |
From @tonycozOn Mon Feb 15 14:46:05 2016, tonyc wrote:
The attached fixes it. Tony |
From @tonycoz0001-move-the-dtrace-macro-calls-out-of-cx_-push-pop-sub.patchFrom b7e538a86b1d07cdcf264cec577664ebb305491d Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Tue, 16 Feb 2016 16:42:09 +1100
Subject: move the dtrace macro calls out of cx_(push|pop)sub()
This avoids references to the internal dtrace symbols by
perlmain when cx_pushsub or cx_popsub are complex enough that
the compiler turns them into static functions (which happens with
-DDEBUGGING.)
---
cop.h | 12 +++++++++---
inline.h | 3 ---
pp_ctl.c | 6 +++---
pp_hot.c | 4 ++--
pp_sort.c | 4 ++--
5 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/cop.h b/cop.h
index dfb4a00..8fb96cb 100644
--- a/cop.h
+++ b/cop.h
@@ -590,11 +590,17 @@ struct block_format {
# define CX_POP(cx) cxstack_ix--;
#endif
+#ifdef PERL_CORE
+
+#define CX_PUSHSUB(cx, cv, retop, hasargs) \
+ PERL_DTRACE_PROBE_ENTRY(cv); \
+ cx_pushsub(cx, cv, retop, hasargs);
-/* base for the next two macros. Don't use directly.
- * The context frame holds a reference to the CV so that it can't be
- * freed while we're executing it */
+#define CX_POPSUB(cx) \
+ PERL_DTRACE_PROBE_RETURN((cx)->blk_sub.cv); \
+ cx_popsub(cx);
+#endif
#define CX_PUSHSUB_GET_LVALUE_MASK(func) \
/* If the context is indeterminate, then only the lvalue */ \
diff --git a/inline.h b/inline.h
index 1e657cc..8916f44 100644
--- a/inline.h
+++ b/inline.h
@@ -480,7 +480,6 @@ S_cx_pushsub(pTHX_ PERL_CONTEXT *cx, CV *cv, OP *retop, bool hasargs)
PERL_ARGS_ASSERT_CX_PUSHSUB;
- PERL_DTRACE_PROBE_ENTRY(cv);
cx->blk_sub.cv = cv;
cx->blk_sub.olddepth = CvDEPTH(cv);
cx->blk_sub.prevcomppad = PL_comppad;
@@ -540,8 +539,6 @@ S_cx_popsub(pTHX_ PERL_CONTEXT *cx)
PERL_ARGS_ASSERT_CX_POPSUB;
assert(CxTYPE(cx) == CXt_SUB);
- PERL_DTRACE_PROBE_RETURN(cx->blk_sub.cv);
-
if (CxHASARGS(cx))
cx_popsub_args(cx);
cx_popsub_common(cx);
diff --git a/pp_ctl.c b/pp_ctl.c
index 7b31bbb..f6cdc8c 100644
--- a/pp_ctl.c
+++ b/pp_ctl.c
@@ -1531,7 +1531,7 @@ Perl_dounwind(pTHX_ I32 cxix)
CX_POPSUBST(cx);
break;
case CXt_SUB:
- cx_popsub(cx);
+ CX_POPSUB(cx);
break;
case CXt_EVAL:
cx_popeval(cx);
@@ -2012,7 +2012,7 @@ PP(pp_dbstate)
}
else {
cx = cx_pushblock(CXt_SUB, gimme, SP, PL_savestack_ix);
- cx_pushsub(cx, cv, PL_op->op_next, 0);
+ CX_PUSHSUB(cx, cv, PL_op->op_next, 0);
/* OP_DBSTATE's op_private holds hint bits rather than
* the lvalue-ish flags seen in OP_ENTERSUB. So cancel
* any CxLVAL() flags that have now been mis-calculated */
@@ -2349,7 +2349,7 @@ PP(pp_leavesublv)
}
CX_LEAVE_SCOPE(cx);
- cx_popsub(cx); /* Stack values are safe: release CV and @_ ... */
+ CX_POPSUB(cx); /* Stack values are safe: release CV and @_ ... */
cx_popblock(cx);
retop = cx->blk_sub.retop;
CX_POP(cx);
diff --git a/pp_hot.c b/pp_hot.c
index 6a280ab..33230f6 100644
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -3647,7 +3647,7 @@ PP(pp_leavesub)
leave_adjust_stacks(oldsp, oldsp, gimme, 0);
CX_LEAVE_SCOPE(cx);
- cx_popsub(cx); /* Stack values are safe: release CV and @_ ... */
+ CX_POPSUB(cx); /* Stack values are safe: release CV and @_ ... */
cx_popblock(cx);
retop = cx->blk_sub.retop;
CX_POP(cx);
@@ -3863,7 +3863,7 @@ PP(pp_entersub)
gimme = GIMME_V;
cx = cx_pushblock(CXt_SUB, gimme, MARK, old_savestack_ix);
hasargs = cBOOL(PL_op->op_flags & OPf_STACKED);
- cx_pushsub(cx, cv, PL_op->op_next, hasargs);
+ CX_PUSHSUB(cx, cv, PL_op->op_next, hasargs);
padlist = CvPADLIST(cv);
if (UNLIKELY((depth = ++CvDEPTH(cv)) >= 2))
diff --git a/pp_sort.c b/pp_sort.c
index c91aab0..3f0b756 100644
--- a/pp_sort.c
+++ b/pp_sort.c
@@ -1671,7 +1671,7 @@ PP(pp_sort)
cx = cx_pushblock(CXt_NULL, gimme, PL_stack_base, old_savestack_ix);
if (!(flags & OPf_SPECIAL)) {
cx->cx_type = CXt_SUB|CXp_MULTICALL;
- cx_pushsub(cx, cv, NULL, hasargs);
+ CX_PUSHSUB(cx, cv, NULL, hasargs);
if (!is_xsub) {
PADLIST * const padlist = CvPADLIST(cv);
@@ -1703,7 +1703,7 @@ PP(pp_sort)
CX_LEAVE_SCOPE(cx);
if (!(flags & OPf_SPECIAL)) {
assert(CxTYPE(cx) == CXt_SUB);
- cx_popsub(cx);
+ CX_POPSUB(cx);
}
else
assert(CxTYPE(cx) == CXt_NULL);
--
1.7.9.2
|
From @iabynOn Mon, Feb 15, 2016 at 09:49:00PM -0800, Tony Cook via RT wrote:
I want to avoid replacing cx_pushsub etc with wrapper macros if at all -- |
From @tonycozOn Tue, Feb 16, 2016 at 12:39:30AM -0800, Dave Mitchell via RT wrote:
That didn't help. The way dtrace linking works is strange. Each object is compiled, just before we link libperl dtrace is run on tony@nereid:~/dev/perl/git/perl$ rm op.o Now since perlmain isn't linked into libperl.so we don't perform that Note that dtrace on Solaris, OS X, FreeBSD and the emulation done by The only way I can see to avoid references to those dtrace symbols is Another option: instead of creating CX_PUSHSUB() and CX_POPSUB() Let me know if you want access to my Solaris VM for testing. Tony * I don't know the details of the translation |
From @iabynOn Wed, Feb 17, 2016 at 09:49:31AM +1100, Tony Cook wrote:
Would it be as simple as wrapping the cx_foo definitions in inline.h with #ifndef PERL_IN_MINIPERLMAIN_C Or are we likely to ru into similar problems when other people try to
Perhaps that would be best, thanks. -- |
From @tonycozOn Wed, Feb 17, 2016 at 08:10:49AM -0800, Dave Mitchell via RT wrote:
That builds successfully. Inline Patchdiff --git a/inline.h b/inline.h
index 1e657cc..acee2ad 100644
--- a/inline.h
+++ b/inline.h
@@ -472,6 +472,7 @@ S_cx_topblock(pTHX_ PERL_CONTEXT *cx)
PL_stack_sp = PL_stack_base + cx->blk_oldsp;
}
+#ifndef PERL_IN_MINIPERLMAIN_C
PERL_STATIC_INLINE void
S_cx_pushsub(pTHX_ PERL_CONTEXT *cx, CV *cv, OP *retop, bool hasargs)
@@ -547,6 +548,7 @@ S_cx_popsub(pTHX_ PERL_CONTEXT *cx)
cx_popsub_common(cx);
}
+#endif
PERL_STATIC_INLINE void
S_cx_pushformat(pTHX_ PERL_CONTEXT *cx, CV *cv, OP *retop, GV *gv)
No, I think it's safe. We're essentially embedding libperl.so into
Please send me a ssh pubkey.
./Configure -des -Dusedevel -Dusedtrace -Duseshrplib -DDEBUGGING -Dmake=gmake && gmake -j2 test-prep hints/solaris.sh picks up the workshop compiler. Tony |
From @iabynI now have a simple standalone example of this failure: With these source files: ==> dtrace.d <== ==> inline.h <== ==> a.c <== ==> main.c <== ==> build <== running ./build gives: ld.so.1: main: fatal: relocation error: file ./liba.so: symbol Looking at symbol tables: $ for i in *.o *.so main; do echo $i; nm $i | grep '$dtrace'; done replacing the dynamic link of main in build with Note that the failure doesn't reply on the equivalent of -DDEBUGGING being What seems to happen is that going from main.o to main loses the I'm a bit stumped at this point. -- |
From @bulk88On Tue Feb 23 06:45:43 2016, davem wrote:
Does the order of the .o files passed to the linker matter in resolving symbols? -- |
From zefram@fysh.orgbulk88 via RT wrote:
No. The linker includes every .o in the link, and all symbols defined -zefram |
From @tonycozOn Tue Feb 23 06:45:43 2016, davem wrote:
The "too big" is in reference to inline functions, your f1() is always static, so the compiler always includes it in the object (though it would be nice if it discarded the unused static function instead.) Solaris is using "static inline": tony@nereid:~/dev/perl/git/perl$ grep static_inline config.sh So my theory was that the extra asserts made S_cx_pushsub() and S_cx_popsub() too large for them to be inlined, so they were built as static per your example.
Unfortunately Oracle's documentation of the how linking and dtrace work is very limited, all I'm aware of is: http://docs.oracle.com/cd/E19253-01/817-6223/chp-usdt-2/index.html which doesn't discuss the mechanisms. Tony |
From @iabynOn Mon, Feb 29, 2016 at 12:14:12AM -0800, Tony Cook via RT wrote:
My stripped-down example fails with or without 'inline' added to the
Yeah, that's all I could find too :-( I guess for now we'll have to go with excluding the S_cx_* inline fns -- |
From @tonycozOn Tue Mar 01 04:46:32 2016, davem wrote:
One issue I see with that is that naughty CPAN code that defines PERL_CORE (like autobox) will see the dtrace probes in the cx_popsub() and cx_pushsub() inline functions in inline.h, and possibly see the same problem. My "add macros" patch avoided that. Tony |
From @tonycozOn Tue Mar 01 04:46:32 2016, davem wrote:
From a private discussion with Alan Burlison, any static linking steps need to include all of the compilation units There were more details relevant to the way dtrace works, but the dtrace maintainers want to keep that private in case they decide to change the implementation in the future. Tony |
From @iabynOn Sun, Mar 06, 2016 at 02:28:41PM -0800, Tony Cook via RT wrote:
I'm currently hacking Makefile.SH to make this work, but have hit a bit of -- |
From @iabynOn Mon, Mar 07, 2016 at 04:48:35PM +0000, Dave Mitchell wrote:
Now pushed for smoking as smoke-me/davem/dtrace4. I Think it would could do with some testing on a non-linux, non-Solaris Also, I haven't been able to see if t/run/dtrace.t works, since it either -- |
From @arcDave Mitchell <davem@iabyn.com> wrote:
I get this output from t/run/dtrace.t on Mac OS 10.9.5 (aka Darwin 13.4.0): run/dtrace.t .. 2/9 # Failed test 3 - phase changes of a simple script Test Summary Report run/dtrace.t (Wstat: 0 Tests: 9 Failed: 5) -- |
From @iabynOn Fri, Mar 11, 2016 at 02:43:35PM +0000, Aaron Crane wrote:
Is that with blead or with my smoke-me/davem/dtrace4 branch? Thanks. -- |
From @arcDave Mitchell <davem@iabyn.com> wrote:
That's with your smoke-me branch.
It also fails for me under both 5.23.7 and blead (output below), but =============== blead 8a1d10a Test Summary Report run/dtrace.t (Wstat: 0 Tests: 9 Failed: 5) =============== v5.23.7 tag Test Summary Report run/dtrace.t (Wstat: 0 Tests: 9 Failed: 4) -- |
From @iabynOn Fri, Mar 11, 2016 at 03:06:36PM +0000, Aaron Crane wrote:
Since the main thrust of my branch was to fix a build/link failure in I've gone ahead and merged this branch, so that 5.24.0 should be no worse -- |
From @iabynOn Sat, Mar 19, 2016 at 12:07:32AM +0000, Dave Mitchell wrote:
It's smoked ok. I'll remove this ticket from 5.24 blockers, -- |
Migrated from rt.perl.org#127543 (status was 'open')
Searchable as RT127543$
The text was updated successfully, but these errors were encountered: