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.

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

ERL-923: 22.0 release candidates cause Yaws test to fail #3787

Closed
OTP-Maintainer opened this issue Apr 26, 2019 · 5 comments
Closed

ERL-923: 22.0 release candidates cause Yaws test to fail #3787

OTP-Maintainer opened this issue Apr 26, 2019 · 5 comments
Assignees
Labels
bug Issue is reported as a bug priority:medium team:VM Assigned to OTP team VM

Comments

@OTP-Maintainer
Copy link

Original reporter: vinoski
Affected version: OTP-22.0
Component: compiler
Migrated from: https://bugs.erlang.org/browse/ERL-923


The Yaws web server supports a binding feature that allows web page content to contain variables that Yaws replaces with values prior to returning the content to the client. The Yaws unit test request_bindings in yaws_compile_SUITE.erl helps verify that Yaws parses variable bindings correctly.

All 3 release candidates for 22.0 fail this test, apparently due to a change in the compiler. I bisected to find the failing change:


{noformat}
commit 7d941c529dc9db016af30cdace2df089d1648dbf
Merge: 08ef38b2c9 afa36d2081
Author: John Högberg <john@erlang.org>
Date:   Fri Sep 28 14:08:23 2018 +0200

    Merge pull request #1958 from jhogberg/john/compiler/ssa-bsm-opt

    Rewrite BSM optimizations in the new SSA-based intermediate format

{noformat}

Within the git branch noted in this merge commit, commit cf5094c, the first one in the branch, passes, but the commits after that on the branch fail.

If I compile Yaws with 21.3 and run it under a 22.0 release candidate, the test does not fail.

Unfortunately, I haven't been able to devise a smaller test case to reproduce this. The approach I've been using is to follow these steps to build Yaws in an Ubuntu docker container (assuming Erlang/OTP is already built/installed there):


{noformat}
    sudo apt-get build-dep yaws
    git clone https://github.com/klacke/yaws.git
    cd yaws
    autoreconf -fi
    ./configure
    make -j8
    sudo make install
{noformat}

I then use erlc to compile this module:


{code:erlang}
-module(comptest).
-export([start/0]).

start() ->
    application:load(yaws),
    application:start(yaws),
    {ok,GC,_} = yaws_api:getconf(),
    put(gc,GC),
    {ok,0,[{data,V}|_]} = yaws_compile:compile_file("comptest-data"),
    application:stop(yaws),
    erlang:halt(case V of
                    5 -> 0;
                    _ -> 1
                end).

{code}

where the input file comptest-data required by the module contains the following:

{noformat}
%%%%<%%x%%>
{noformat}

After compiling the module, I run it with


{noformat}
erl -noshell -noinput -s comptest start
{noformat}

If this exits with a 0 status, the test passed.

The error occurs within the Yaws source file src/yaws_compile.erl. From what I can tell, something is compiled incorrectly within the many clauses of yaws_compile:compile_file/4, particularly those involving yaws_compile:parse_binding/1,2.
@OTP-Maintainer
Copy link
Author

john said:

Thanks for your report, I've narrowed down the issue and hope to have it resolved early next week.

@OTP-Maintainer
Copy link
Author

vinoski said:

Great, thanks! Once you have a branch ready, I'd be happy to test it. Let me know when it's available and I'll pull it and try it out.

@OTP-Maintainer
Copy link
Author

john said:

https://github.com/jhogberg/otp/tree/john/compiler/fix-missing-match-reposition/ERL-923

There's a lot of polish left (I'll get around to that on Monday), but it ought to work.

@OTP-Maintainer
Copy link
Author

vinoski said:

Thanks, I verified the failing Yaws test case now passes against your branch.

@OTP-Maintainer
Copy link
Author

john said:

I've merged a fix to master, thanks again for your report!

@OTP-Maintainer OTP-Maintainer added bug Issue is reported as a bug team:VM Assigned to OTP team VM priority:medium labels Feb 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is reported as a bug priority:medium team:VM Assigned to OTP team VM
Projects
None yet
Development

No branches or pull requests

2 participants