Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

BBC: Blead Breaks HTML::Mason #20291

Closed
cjg-cguevara opened this issue Sep 12, 2022 · 5 comments
Closed

BBC: Blead Breaks HTML::Mason #20291

cjg-cguevara opened this issue Sep 12, 2022 · 5 comments
Labels
BBC Blead Breaks CPAN - changes in blead broke a cpan module(s)

Comments

@cjg-cguevara
Copy link

This is a bug report for perl from "Carlos Guevara" carlos@carlosguevara.com,
generated with the help of perlbug 1.43 running under perl 5.37.4.

BBC: Blead Breaks HTML::Mason

Please see http://matrix.cpantesters.org/?dist=HTML::Mason

Flags

  • category=core
  • severity=low

Perl configuration

Site configuration information for perl 5.37.4:

Configured by cpan at Mon Sep 12 10:08:25 EDT 2022.

Summary of my perl5 (revision 5 version 37 subversion 4) configuration:
  Commit id: dcf2c66b4ba2e50408461d7ac13a4782ba26bf1b
  Platform:
    osname=linux
    osvers=5.15.64-0-lts
    archname=x86_64-linux-thread-multi
    uname='linux cjg-alpine3 5.15.64-0-lts #1-alpine smp mon, 05 sep 2022 08:02:49 +0000 x86_64 linux '
    config_args='-des -Dprefix=~/bin/perl-blead -Dscriptdir=~/bin/perl-blead/bin -Dusedevel -Duse64bitall -Duseithreads'
    hint=recommended
    useposix=true
    d_sigaction=define
    useithreads=define
    usemultiplicity=define
    use64bitint=define
    use64bitall=define
    uselongdouble=undef
    usemymalloc=n
    default_inc_excludes_dot=define
  Compiler:
    cc='cc'
    ccflags ='-D_REENTRANT -D_GNU_SOURCE -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'
    optimize='-O2'
    cppflags='-D_REENTRANT -D_GNU_SOURCE -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong'
    ccversion=''
    gccversion='11.2.1 20220219'
    gccosandvers=''
    intsize=4
    longsize=8
    ptrsize=8
    doublesize=8
    byteorder=12345678
    doublekind=3
    d_longlong=define
    longlongsize=8
    d_longdbl=define
    longdblsize=16
    longdblkind=3
    ivtype='long'
    ivsize=8
    nvtype='double'
    nvsize=8
    Off_t='off_t'
    lseeksize=8
    alignbytes=8
    prototype=define
  Linker and Libraries:
    ld='cc'
    ldflags =' -fstack-protector-strong -L/usr/local/lib'
    libpth=/usr/lib /usr/local/lib /lib
    libs=-lpthread -ldb -ldl -lm -lcrypt -lutil -lc
    perllibs=-lpthread -ldl -lm -lcrypt -lutil -lc
    libc=/usr/lib/libc.a
    so=so
    useshrplib=false
    libperl=libperl.a
    gnulibc_version=''
  Dynamic Linking:
    dlsrc=dl_dlopen.xs
    dlext=so
    d_dlsymun=undef
    ccdlflags='-Wl,-E'
    cccdlflags='-fPIC'
    lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector-strong'


---
@INC for perl 5.37.4:
    /home/cpan/bin/perl-blead/lib/site_perl/5.37.4/x86_64-linux-thread-multi
    /home/cpan/bin/perl-blead/lib/site_perl/5.37.4
    /home/cpan/bin/perl-blead/lib/5.37.4/x86_64-linux-thread-multi
    /home/cpan/bin/perl-blead/lib/5.37.4

---
Environment for perl 5.37.4:
    HOME=/home/cpan
    LANG=C.UTF-8
    LANGUAGE (unset)
    LC_COLLATE=C
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/cpan/bin/perl-blead/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
    PERL_BADLANG (unset)
    SHELL=/bin/bash
@jkeenan
Copy link
Contributor

jkeenan commented Sep 12, 2022 •
edited

I have reproduced this breakage by building perl at 5d56ab3 and attempting to install HTML::Mason against it. The breakage in t/13-errors.t looks like this:

$ uname -mrs
FreeBSD 12.3-RELEASE-p1 amd64
$ bleadperl -v | head -2 | tail -1
This is perl 5, version 37, subversion 4 (v5.37.4 (v5.37.3-297-g5d56ab3b35)) built for amd64-freebsd-thread-multi
$ bleadperl -V:config_args
config_args='-des -Dusedevel -Uversiononly -Dprefix=/home/jkeenan/testing/blead -Dman1dir=none -Dman3dir=none -Duseithreads -Doptimize=-O2 -pipe -fstack-protector -fno-strict-aliasing';
$ bleadprove -vb t/13-errors.t
...
ok 19 - content_comp_wrong_error
# Running top_level_compilation_error (#20): Make sure top-level compiler errors work in output mode
# Got ...
# -----
# Error during compilation of /usr/home/jkeenan/.cpan/build/HTML-Mason-1.59-0/mason_tests/22823/comps/errors/top_level_compilation_error:
# syntax error at /usr/home/jkeenan/.cpan/build/HTML-Mason-1.59-0/mason_tests/22823/comps/errors/top_level_compilation_error line 2, near ";"
# 
# Stack:
#   [/usr/home/jkeenan/.cpan/build/HTML-Mason-1.59-0/mason_tests/22823/comps/errors/top_level_compilation_error:2]
#   [/usr/home/jkeenan/.cpan/build/HTML-Mason-1.59-0/blib/lib/HTML/Mason/Interp.pm:817]
#   [/usr/home/jkeenan/.cpan/build/HTML-Mason-1.59-0/blib/lib/HTML/Mason/Interp.pm:445]
#   [/usr/home/jkeenan/.cpan/build/HTML-Mason-1.59-0/blib/lib/HTML/Mason/Request.pm:250]
#   [/usr/home/jkeenan/.cpan/build/HTML-Mason-1.59-0/blib/lib/HTML/Mason/Request.pm:212]
#   [/home/jkeenan/testing/blead/lib/perl5/site_perl/5.37.4/Class/Container.pm:275]
#   [/home/jkeenan/testing/blead/lib/perl5/site_perl/5.37.4/Class/Container.pm:353]
#   [/usr/home/jkeenan/.cpan/build/HTML-Mason-1.59-0/blib/lib/HTML/Mason/Interp.pm:348]
#   [/usr/home/jkeenan/.cpan/build/HTML-Mason-1.59-0/blib/lib/HTML/Mason/Interp.pm:342]
#   [/usr/home/jkeenan/.cpan/build/HTML-Mason-1.59-0/blib/lib/HTML/Mason/Tests.pm:528]
#   [/usr/home/jkeenan/.cpan/build/HTML-Mason-1.59-0/blib/lib/HTML/Mason/Tests.pm:515]
#   [/usr/home/jkeenan/.cpan/build/HTML-Mason-1.59-0/blib/lib/HTML/Mason/Tests.pm:456]
#   [/usr/home/jkeenan/.cpan/build/HTML-Mason-1.59-0/blib/lib/HTML/Mason/Tests.pm:263]
#   [t/13-errors.t:12]
# 
# 
# Stack:
#   [/usr/home/jkeenan/.cpan/build/HTML-Mason-1.59-0/blib/lib/HTML/Mason/Interp.pm:450]
#   [/usr/home/jkeenan/.cpan/build/HTML-Mason-1.59-0/blib/lib/HTML/Mason/Request.pm:250]
#   [/usr/home/jkeenan/.cpan/build/HTML-Mason-1.59-0/blib/lib/HTML/Mason/Request.pm:212]
#   [/home/jkeenan/testing/blead/lib/perl5/site_perl/5.37.4/Class/Container.pm:275]
#   [/home/jkeenan/testing/blead/lib/perl5/site_perl/5.37.4/Class/Container.pm:353]
#   [/usr/home/jkeenan/.cpan/build/HTML-Mason-1.59-0/blib/lib/HTML/Mason/Interp.pm:348]
#   [/usr/home/jkeenan/.cpan/build/HTML-Mason-1.59-0/blib/lib/HTML/Mason/Interp.pm:342]
#   [/usr/home/jkeenan/.cpan/build/HTML-Mason-1.59-0/blib/lib/HTML/Mason/Tests.pm:528]
#   [/usr/home/jkeenan/.cpan/build/HTML-Mason-1.59-0/blib/lib/HTML/Mason/Tests.pm:515]
#   [/usr/home/jkeenan/.cpan/build/HTML-Mason-1.59-0/blib/lib/HTML/Mason/Tests.pm:456]
#   [/usr/home/jkeenan/.cpan/build/HTML-Mason-1.59-0/blib/lib/HTML/Mason/Tests.pm:263]
#   [t/13-errors.t:12]
# 
# -----
#    ... but expected ...
# -----
# (?^s:Error during compilation((?!Stack:).)*Stack:((?!Stack:).)*$)
# -----
not ok 20 - top_level_compilation_error

#   Failed test 'top_level_compilation_error'
#   at /usr/home/jkeenan/.cpan/build/HTML-Mason-1.59-0/blib/lib/HTML/Mason/Tests.pm line 593.
# Running component_error_handler_false (#21): Test error-handling with component_error_handler set to false
ok 21 - component_error_handler_false

The failing test looks like this:

    $group->add_test( name => 'top_level_compilation_error',
                      description => "Make sure top-level compiler errors work in output mode",
                      interp_params => {
                                         error_format => 'text',
                                         error_mode => 'output',
                                       },
                      component => <<'EOF',
% my $x = 
EOF
                        # match "Error during compilation" followed by 
                        # exactly one occurance of "Stack:"
                        # (Mason should stop after the first error)
                      expect => qr/Error during compilation((?!Stack:).)*Stack:((?!Stack:).)*$/s,
                    );

I have not bisected, but I strongly suspect this failure is associated with eb54d46.

commit eb54d46f7264ff7af62c409d8a6ab984a5a34f57
Author:     Yves Orton <demerphq@gmail.com>
AuthorDate: Fri Aug 26 16:26:14 2022
Commit:     Yves Orton <demerphq@gmail.com>
CommitDate: Fri Sep 9 16:48:52 2022

    Stop parsing on first syntax error.

#20168

@demerphq, can you take a look?

@jkeenan jkeenan added BBC Blead Breaks CPAN - changes in blead broke a cpan module(s) and removed Needs Triage labels Sep 12, 2022
@jkeenan
Copy link
Contributor

jkeenan commented Sep 12, 2022

Bisection has confirmed that this is the first bad commit.

commit eb54d46f7264ff7af62c409d8a6ab984a5a34f57 (refs/bisect/bad)
Author:     Yves Orton <demerphq@gmail.com>
AuthorDate: Fri Aug 26 18:26:14 2022 +0200
Commit:     Yves Orton <demerphq@gmail.com>
CommitDate: Fri Sep 9 18:48:52 2022 +0200

    Stop parsing on first syntax error.

@demerphq
Copy link
Collaborator

demerphq commented Sep 13, 2022 •
edited

Executive summary: Not a bug in perl. HTML::Mason::Interp doesn't handle that $@ may be a string or an object, depending on whether it fails with a die or not.

This is a tangled web of issues. The summary is that this may highlight a bug in Perl, but it does not highlight a bug in the "stop on first error". This bug may be demonstrated by modifying the code to contain 10 (or 11, it seems to vary depending on the specific errors encountered) errors instead of 1 even on older perls. The difference between 10 and 1 is that if the compiler "runs out of source code" with less than 10 errors inside of an `eval STRING`, the compilation fails, and the compilation error is appended to `$@`, but crucially, `$SIG{__DIE__}` is NOT triggered. With 10 errors in older perls `$SIG{__DIE__}` *will* be triggered. In newer perl's the first syntax error triggers a croak, which triggers `$SIG{__DIE__}`.

This in turn exposes a bug in HTML::Mason::Interp which does this:

$err = $warnstr . $@;

When eval compilation fails with a die (Perl_croak internally) then $@ will contain an object because there is a $SIG{__DIE__} handler in place. The concatenation joins the warn strings with the object and results in a string containing the Stack: part, this is then fed into compilation_error() which I guess turns it back into an object and adds another Stack: section (as far as I can tell, i stopped debugging when I hit Exception::Class and the concatenation.)

However the point here for me is that if I change the test so that compilation /dies/ instead of "running off the end of source before encountering enough compilation errors" for instance with this change to t/013-errors.pl:

-% my $x = 
+% BEGIN{ die "bad mojo" }

you will see the same failure. If I then change the concatenation to be:

-    $err = $warnstr . $@;
+    $err = length($warnstr) ? $warnstr . $@ : $@;

the error will go away. It wont be entirely fixed, this just avoids the concatenation if there is no warning. But if the code ALSO warned, it would still fail test. Eg,

-% my $x = 
+% BEGIN{warn "you have been warned" } BEGIN{ die "bad mojo" }

would fail again.

So basically, the "stop on first error" fix exposes the fact that $SIG{__DIE__} has historically only sometimes been called on syntax errors, because it is now always called on syntax errors. The underlying issues remains. If less than 10 non-syntax errors are encountered during a parse then $SIG{__DIE__} will not be called, and if 10 or more are encountered it will be.

This then exposes the fact that the code in HTML::Mason::Interp improperly joins a warning string with an exception object when $SIG{__DIE__} is triggered when it evals something.

Now, whether $SIG{__DIE__} should be called when there are errors during a compile is a somewhat open question. I personally would say yes. But historically it has not been consistently called, only when the code had 10 or more errors. I would say this inconsistency is a bug and we should be calling $SIG{__DIE__} always when there is a compilation error. Also there is an open question if $SIG{DIE} should be called on each individual error one at a time, or on the accumulated list of errors at the end.

However, this also means that this is a long existent bug and does not have to be fixed with haste. The stop on first error just exposes the issues more clearly, but it did not cause them.

The underlying perl issue can be seen here:

$ perl -le'print $]; $SIG{__DIE__}= sub { warn "in __DIE__: $_[0]"; $_[0] }; eval "my $x=;"; print $@;'
5.037004
in __DIE__: syntax error at (eval 1) line 1, near "my ="
syntax error at (eval 1) line 1, near "my ="

compare with:

$ perl -le'print $]; $SIG{__DIE__}= sub { warn "in __DIE__: $_[0]"; $_[0] }; eval "my $x=;"; print $@;'
5.034001
syntax error at (eval 1) line 1, near "my ="

Notice the $SIG{__DIE__} was not triggered. Change the expression to something with 10 (or 11) errors:

$ perl -le'print $]; $SIG{__DIE__}= sub { warn "in __DIE__: $_[0]"; $_[0] }; eval( "my $x=;" x 11); print $@;'
5.034001
in __DIE__: syntax error at (eval 1) line 1, near "my ="
Can't redeclare "my" in "my" at (eval 1) line 1, near ";my"
Can't redeclare "my" in "my" at (eval 1) line 1, near ";my"
Can't redeclare "my" in "my" at (eval 1) line 1, near ";my"
Can't redeclare "my" in "my" at (eval 1) line 1, near ";my"
Can't redeclare "my" in "my" at (eval 1) line 1, near ";my"
Can't redeclare "my" in "my" at (eval 1) line 1, near ";my"
Can't redeclare "my" in "my" at (eval 1) line 1, near ";my"
Can't redeclare "my" in "my" at (eval 1) line 1, near ";my"
Can't redeclare "my" in "my" at (eval 1) line 1, near ";my"
(eval 1) has too many errors.
syntax error at (eval 1) line 1, near "my ="
Can't redeclare "my" in "my" at (eval 1) line 1, near ";my"
Can't redeclare "my" in "my" at (eval 1) line 1, near ";my"
Can't redeclare "my" in "my" at (eval 1) line 1, near ";my"
Can't redeclare "my" in "my" at (eval 1) line 1, near ";my"
Can't redeclare "my" in "my" at (eval 1) line 1, near ";my"
Can't redeclare "my" in "my" at (eval 1) line 1, near ";my"
Can't redeclare "my" in "my" at (eval 1) line 1, near ";my"
Can't redeclare "my" in "my" at (eval 1) line 1, near ";my"
Can't redeclare "my" in "my" at (eval 1) line 1, near ";my"
(eval 1) has too many errors.

and $SIG{__DIE__} gets called.

IMO this ticket should be closed and a ticket created for the underlying HTML::Mason bug, it clearly does not handle a preexisting mode of behavior that will now be more common. We should open a new ticket that $SIG{__DIE__} does not get reliably called during compilation.

@jkeenan jkeenan mentioned this issue Sep 13, 2022
Closed
This was referenced Sep 25, 2022
Open
Merged
@jkeenan jkeenan mentioned this issue Oct 17, 2022
Open
@jkeenan
Copy link
Contributor

jkeenan commented Oct 17, 2022

I have submitted a naive p.r. to HTML-Mason to get the failing test to pass:
houseabsolute/HTML-Mason#34

@jkeenan
Copy link
Contributor

jkeenan commented Feb 11, 2023

Fixed upstream; see https://metacpan.org/dist/HTML-Mason. Closing ticket. Thanks.

@jkeenan jkeenan closed this as completed Feb 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
BBC Blead Breaks CPAN - changes in blead broke a cpan module(s)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jkeenan @demerphq @cjg-cguevara