My first Erlang patch

July 20, 2016

Example of how you can contribute to Erlang by fixing a bug and having the fix committed to the Erlang source tree.

Closeup of bright red bug on deep green leaf. class=
© 2016 Ritchie Valens for Unsplash

Finding the bug.

I’ve written a wrapper around Erlang’s erl_tidy module that reads stdin and writes the formatted result to stdout. While learning Go, I got used to gofmt and wanted the same tool for Erlang. When I used my erlfmt utility on some Erlang code I was writing, it failed.

I cut the input down until I found a small input that would trigger the crash:

$ cat t.erl
-module(t).

-export([start/0]).

start() -> io:put_chars("~").
$

Fixing the bug.

The error was badarg on line 299.

** exception exit: badarg
     in function  erl_tidy:file/2 (erl_tidy.erl, line 299)

What is on line 299?

Line 299 in erl_tidy.erl exits and outputs the error reason returned by the child process.

$ (cd $HOME/src ; git clone https://github.com/erlang/otp.git)
$ vi $(find $HOME/src/otp -name erl_tidy.erl)

 292 file(Name, Opts) ->
 293     Parent = self(),
 294     Child = spawn_link(fun () -> file_1(Parent, Name, Opts) end),
 295     receive
 296         {Child, ok} ->
 297             ok;
 298         {Child, {error, Reason}} ->
 299             exit(Reason)
 300     end.

Copy erl_tidy module locally and verify same error.

I copied it locally so I could edit the file …

$ cp $(find $HOME/src/otp -name erl_tidy.erl) .
$ cp erl_tidy.erl erl_tidy.orig

… and started by verifying that my local copy gave the same error.

$ erl
Erlang/OTP 19 [erts-8.0.1] [source-ca40008] [64-bit] [smp:8:8] [async-threads:10] [hipe] [kernel-poll:false]

Eshell V8.0.1  (abort with ^G)
1> c(erl_tidy).
{ok,erl_tidy}
2> erl_tidy:file("t.erl", [{stdout,true}]).
** exception exit: badarg
     in function  erl_tidy:file/2 (erl_tidy.erl, line 299)
3>

Figure out which line of client code is generating the error.

Since the error is passed back from a spawned process to the parent process, we don’t know the line of client code that has the problem. So, I sprinkled some io:put_char() statements to figure out where it crashes.

I knew it was related to the {stdout,true} argument since I could tidy the file without error if I did not use that option. That helped me narrow down where to put the debug statements.

$ vi erl_tidy.erl

$ diff -uw erl_tidy.orig erl_tidy.erl | grep -n ^
1:— erl_tidy.orig    2016-07-16 09:11:41.000000000 -0400
2:+++ erl_tidy.erl    2016-07-16 11:14:58.000000000 -0400
3:@@ -300,6 +300,7 @@
4:     end.
5: 
6: file_1(Parent, Name, Opts) ->
7:+    io:put_chars("MKB: enter file_1\n"),
8:     try file_2(Name, Opts) of
9:     _ ->
10:         Parent ! {self(), ok}
11:@@ -311,6 +312,7 @@
12:     end.
13: 
14: file_2(Name, Opts) ->
15:+    io:put_chars("MKB: enter file_2\n"),
16:     Opts1 = Opts ++ file__defaults(),
17:     Forms = read_module(Name, Opts1),
18:     Comments = erl_comment_scan:file(Name),
19:@@ -417,7 +419,9 @@
20:     end.
21: 
22: print_module(Tree, Opts) ->
23:+     io:put_chars("MKB: print_module - 1\n"),
24:     Printer = proplists:get_value(printer, Opts),
25:+     io:put_chars("MKB: print_module - 2\n"),
26:     io:format(Printer(Tree, Opts)).
27: 
28: output(FD, Printer, Tree, Opts) ->

Run the instrumented code to see how far it gets.

$ erl
Erlang/OTP 19 [erts-8.0.1] [source-ca40008] [64-bit] [smp:8:8] [async-threads:10] [hipe] [kernel-poll:false]

Eshell V8.0.1  (abort with ^G)
1> c(erl_tidy).
{ok,erl_tidy}
2> erl_tidy:file("t.erl", [{stdout,true}]).
MKB: enter file_1
MKB: enter file_2
MKB: print_module - 1
MKB: print_module - 2
** exception exit: badarg
     in function  erl_tidy:file/2 (erl_tidy.erl, line 299)
3>

The program outputs "MKB: print_module - 2" and then raises the error, so the next line is the culprit:

26:     io:format(Printer(Tree, Opts)).

Is it io:format() or Printer()?

There are two calls on that line, so unwind them and insert another io:put_chars: Line numbers here are of erl_tidy.

 421 print_module(Tree, Opts) ->
 422     io:put_chars("MKB: print_module - 1\n"),
 423     Printer = proplists:get_value(printer, Opts),
 424     io:put_chars("MKB: print_module - 2\n"),
 425     Text = Printer(Tree, Opts), 
 426     io:format("MKB: print_module - 3: Text = ~p\n", [Text]),
 427     io:format(Text).

When I run this, the output changes: lines 425 and 426 execute successfully, so the problem must be on line 427 in the io:format() call.

$ erl
Erlang/OTP 19 [erts-8.0.1] [source-ca40008] [64-bit] [smp:8:8] [async-threads:10] [hipe] [kernel-poll:false]

Eshell V8.0.1  (abort with ^G)
1> c(erl_tidy).
{ok,erl_tidy}
2> erl_tidy:file("t.erl", [{stdout,true}]).
MKB: enter file_1
MKB: enter file_2
MKB: print_module - 1
MKB: print_module - 2
MKB: print_module - 3: Text = "-module(t).\n\n-export([start/0]).\n\nstart() -> io:put_chars(\"~\")."
** exception exit: badarg
     in function  erl_tidy:file/2 (erl_tidy.erl, line 299)
3>

The fix.

After some staring at the "print_module - 3" line above, the problem is clear. The tilde character is the format identifier character in Erlang, as the percent character is in C.

The problem can be duplicated most concisely in the console like this:

4> io:format("~").                                                                    
** exception error: bad argument
     in function  io:format/3
        called as io:format(<0.50.0>,"~",[])
5> 

Since the intent is to print a string, changing io:format() to io:put_chars() fixes the bug.

Committing the fix to the Erlang source tree.

Erlang uses the normal github process for pull requests.

The Erlang team provides a well-written set of instructions on how to submit a patch.

It’s pretty much the normal github flow, with a couple wrinkles:

  1. use git config to setup your name and email
  2. for bug fixes, patch the maint branch
  3. test first
  4. write a good commit message
  5. clean up your patch before submitting pull request

Building Erlang and running the syntax_tools test suite.

Again, great docs: INSTALL.md and TESTING.md.

$ git clone git@github.com:mbucc/otp.git
$ cd otp
$ export ERL_TOP=‘pwd‘
$ ./otp_build autoconf  # (6s)
$ ./otp_build setup -a  # (11m)
$ export PATH=‘pwd‘/bin:$PATH
$ ./otp_build tests     # (1m)
$ cd lib/syntax_tools/
$ ct_run -pa ../../release/tests/test_server/ -suite ./test/syntax_tools_SUITE
Erlang/OTP 19 [erts-8.0.1] [source-ca40008] [64-bit] [smp:8:8] [async-threads:10] [hipe] [kernel-poll:false]

Converting "../../release/tests/test_server" to "/Users/mark/src/otp/release/tests/test_server" and re-inserting with add_patha/1


Common Test starting (cwd is /Users/mark/src/otp/lib/syntax_tools)

Eshell V8.0.1  (abort with ^G)
(ct@marks-mbp-3)1> 
Common Test: Running make in test directories…
Recompile: syntax_tools_SUITE

CWD set to: "/Users/mark/src/otp/lib/syntax_tools/ct_run.ct@marks-mbp-3.2016-07-19_07.24.34"

TEST INFO: 1 test(s), 11 case(s) in 1 suite(s)

Testing lib.syntax_tools.syntax_tools_SUITE: Starting test, 11 test cases
Testing lib.syntax_tools.syntax_tools_SUITE: TEST COMPLETE, 11 ok, 0 failed of 11 test cases

Updating /Users/mark/src/otp/lib/syntax_tools/index.html … done
Updating /Users/mark/src/otp/lib/syntax_tools/all_runs.html … done

$

However, this is not the way they tell you to do it!

The TESTING.md instructions say to use the ts() module. However, that approach does not notice and recompile changed files. The ct_run approach above does.

If you do use the ts() module, and you are on OSX, use the "batch" option so XQuartz doesn’t open; for example,

1> ts:run(syntax_tools, syntax_tools_SUITE, [batch]).

or

1> ts:run(syntax_tools, syntax_tools_SUITE, t_erl_tidy, [verbose, batch]).

Final notes

The pull request is https://github.com/erlang/otp/pull/1071 and the resulting commit is https://github.com/erlang/otp/commit/0969ecad9e2978bacb7b27f663cd16aa5ea6c460. Note that in this case, I was too slow with the git squash (aka rebase) and Hans Bolinder did it for me.

It took less that a month to get committed, and that was delayed because the Erlang team was in the middle of a release.


If you see an error or something that could be improved, please let me know. This is a blog about me learning, so I expect I will get some stuff wrong. The best way to reach me is by email: mkbucc1234@gmail.com (after deleting all the numbers).

To make a comment, check for a thread on the erlang subreddit and if there isn't one, then start one up.

Follow on Twitter: @mbucc

Back to the index.