Skip to content

Commit 5b01f19

Browse files
committed
Reapply squashed "avoid identical stack traces" patches
This reapplies two reverted patches as a single squashed commit. Revert "Revert "avoid identical stack traces"" This reverts commit 79f75ea which itself reverted f2f32cd Revert "Revert "fixup to "avoid identical stack traces" - try 2"" This reverts commit 2abf7ef which itself reverted ad89278 Original Author: David Mitchell <[email protected]> Original Date: Fri Dec 13 13:48:25 2019 +0000 avoid identical stack traces GH #15109 The output of caller() (e.g. as produced by carp::Confess) produces multiple identical outputs when within a nested use/require. This is because at the time of calling the 'BEGIN { require ... }', PL_curcop is set to &PL_compiling, which is a fixed buffer within the interpreter, whose individual file and line fields are saved and restored when doing a new require/eval. This means that within the innermost require, PL_compiling has file:lineno of the innermost source file, and multiple saved PL_curcop values in the context stack frames all point to the same &PL_copmpiling. So all levels of the stack trace appear to come from the innermost file. This commit fixes this (after a fashion) by, at the start of calling a BEGIN, making PL_curcop point to a temporary copy of PL_compiling instead. This is all a bit of a hack.
1 parent 91055b9 commit 5b01f19

File tree

7 files changed

+87
-6
lines changed

7 files changed

+87
-6
lines changed

MANIFEST

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5682,6 +5682,10 @@ t/lib/feature/nonesuch Tests for enabling/disabling nonexistent feature
56825682
t/lib/feature/removed Tests for enabling/disabling removed feature
56835683
t/lib/feature/say Tests for enabling/disabling say feature
56845684
t/lib/feature/switch Tests for enabling/disabling switch feature
5685+
t/lib/GH_15109/Apack.pm test Module for caller.t
5686+
t/lib/GH_15109/Bpack.pm test Module for caller.t
5687+
t/lib/GH_15109/Cpack.pm test Module for caller.t
5688+
t/lib/GH_15109/Foo.pm test Module for caller.t
56855689
t/lib/h2ph.h Test header file for h2ph
56865690
t/lib/h2ph.pht Generated output from h2ph.h by h2ph, for comparison
56875691
t/lib/locale/latin1 Part of locale.t in Latin 1

op.c

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12043,10 +12043,31 @@ S_process_special_blocks(pTHX_ I32 floor, const char *const fullname,
1204312043
(void)CvGV(cv);
1204412044
if (floor) LEAVE_SCOPE(floor);
1204512045
ENTER;
12046+
12047+
SAVEVPTR(PL_curcop);
12048+
if (PL_curcop == &PL_compiling) {
12049+
/* Avoid pushing the "global" &PL_compiling onto the
12050+
* context stack. For example, a stack trace inside
12051+
* nested use's would show all calls coming from whoever
12052+
* most recently updated PL_compiling.cop_file and
12053+
* cop_line. So instead, temporarily set PL_curcop to a
12054+
* private copy of &PL_compiling. PL_curcop will soon be
12055+
* set to point back to &PL_compiling anyway but only
12056+
* after the temp value has been pushed onto the context
12057+
* stack as blk_oldcop.
12058+
* This is slightly hacky, but necessary. Note also
12059+
* that in the brief window before PL_curcop is set back
12060+
* to PL_compiling, IN_PERL_COMPILETIME/IN_PERL_RUNTIME
12061+
* will give the wrong answer.
12062+
*/
12063+
PL_curcop = (COP*)newSTATEOP(PL_compiling.op_flags, NULL, NULL);
12064+
CopLINE_set(PL_curcop, CopLINE(&PL_compiling));
12065+
SAVEFREEOP(PL_curcop);
12066+
}
12067+
1204612068
PUSHSTACKi(PERLSI_REQUIRE);
1204712069
SAVECOPFILE(&PL_compiling);
1204812070
SAVECOPLINE(&PL_compiling);
12049-
SAVEVPTR(PL_curcop);
1205012071

1205112072
DEBUG_x( dump_sub(gv) );
1205212073
Perl_av_create_and_push(aTHX_ &PL_beginav, MUTABLE_SV(cv));

t/lib/GH_15109/Apack.pm

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
# for use by caller.t for GH #15109
2+
package Apack;
3+
use Bpack;
4+
1;

t/lib/GH_15109/Bpack.pm

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
# for use by caller.t for GH #15109
2+
package Bpack;
3+
use Cpack;
4+
1;

t/lib/GH_15109/Cpack.pm

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# for use by caller.t for GH #15109
2+
package Cpack;
3+
4+
5+
my $i = 0;
6+
7+
while (my ($package, $file, $line) = caller($i++)) {
8+
push @Cpack::callers, "$file:$line";
9+
}
10+
11+
1;

t/lib/GH_15109/Foo.pm

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
# for use by caller.t for GH #15109
2+
3+
package Foo;
4+
5+
sub import {
6+
use warnings; # restore default warnings
7+
() = caller(1); # this used to cause valgrind errors
8+
}
9+
1;

t/op/caller.t

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ BEGIN {
55
chdir 't' if -d 't';
66
require './test.pl';
77
set_up_inc('../lib');
8-
plan( tests => 97 ); # some tests are run in a BEGIN block
8+
plan( tests => 111 ); # some tests are run in a BEGIN block
99
}
1010

1111
my @c;
@@ -335,16 +335,44 @@ $::testing_caller = 1;
335335

336336
do './op/caller.pl' or die $@;
337337

338+
# GH #15109
339+
# See that callers within a nested series of 'use's gets the right
340+
# filenames.
341+
{
342+
local @INC = 'lib/GH_15109/';
343+
# Apack use's Bpack which use's Cpack which populates @Cpack::caller
344+
# with the file:N of all the callers
345+
eval 'use Apack; 1';
346+
is($@, "", "GH #15109 - eval");
347+
is (scalar(@Cpack::callers), 10, "GH #15109 - callers count");
348+
like($Cpack::callers[$_], qr{GH_15109/Bpack.pm:3}, "GH #15109 level $_") for 0..2;
349+
like($Cpack::callers[$_], qr{GH_15109/Apack.pm:3}, "GH #15109 level $_") for 3..5;
350+
like($Cpack::callers[$_], qr{\(eval \d+\):1}, "GH #15109 level $_") for 6..8;
351+
like($Cpack::callers[$_], qr{caller\.t}, "GH #15109 level $_") for 9;
352+
353+
# GH #15109 followup - the original fix wasn't saving cop_warnings
354+
# correctly and this code used to crash or fail valgrind
355+
356+
my $w = 0;
357+
local $SIG{__WARN__} = sub { $w++ };
358+
eval q{
359+
use warnings;
360+
no warnings 'numeric'; # ensure custom cop_warnings
361+
use Foo; # this used to mess up warnings flags
362+
BEGIN { my $x = "foo" + 1; } # potential "numeric" warning
363+
};
364+
is ($@, "", "GH #15109 - eval okay");
365+
is ($w, 0, "GH #15109 - warnings restored");
366+
}
367+
338368
{
339369
package RT129239;
340370
BEGIN {
341371
my ($pkg, $file, $line) = caller;
342372
::is $file, 'virtually/op/caller.t', "BEGIN block sees correct caller filename";
343373
::is $line, 12345, "BEGIN block sees correct caller line";
344-
TODO: {
345-
local $::TODO = "BEGIN blocks have wrong caller package [perl #129239]";
346-
::is $pkg, 'RT129239', "BEGIN block sees correct caller package";
347-
}
374+
::is $pkg, 'RT129239', "BEGIN block sees correct caller package";
348375
#line 12345 "virtually/op/caller.t"
349376
}
377+
350378
}

0 commit comments

Comments
 (0)