TritonDataCenter#35 clean up interfaces for working with functions an…

…d strings

.gitignore

@@ -1 +1,2 @@
 build
+tools/mdbv8diff/node_modules

CHANGES.md

@@ -13,6 +13,7 @@
 ## Unreleased changes
 
 * #48 want tool for running tests on multiple Node versions
+* #35 clean up interfaces for working with functions and strings
 
 ## v1.1.1 (2015-10-02)
 

GNUmakefile

@@ -46,7 +46,14 @@ MDBV8_VERS_TAG	 = "dev"
 # List of source files that will become objects.  (These entries do not include
 # the "src/" directory prefix.)
 #
-MDBV8_SOURCES		 = mdb_v8.c mdb_v8_cfg.c mdb_v8_context.c
+MDBV8_SOURCES		 = \
+    mdb_v8.c \
+    mdb_v8_cfg.c \
+    mdb_v8_function.c \
+    mdb_v8_strbuf.c \
+    mdb_v8_string.c \
+    mdb_v8_subr.c
+
 MDBV8_GENSOURCES	 = mdb_v8_version.c
 
 # List of source files to run through cstyle.  This includes header files.
@@ -78,7 +85,10 @@ CSTYLE_FLAGS		+= -cCp
 CATEST			 = tools/catest
 
 # JavaScript source files (used in test code)
-JS_FILES		 = $(wildcard test/standalone/*.js)
+JS_FILES		 = $(wildcard test/standalone/*.js) \
+			   $(wildcard tools/mdbv8diff/*.js) \
+			   $(wildcard tools/mdbv8diff/issues/*.js) \
+			   tools/mdbv8diff/mdbv8diff
 JSL_FILES_NODE		 = $(JS_FILES)
 JSSTYLE_FILES		 = $(JS_FILES)
 JSL_CONF_NODE		 = tools/jsl.node.conf

README.md

@@ -61,9 +61,7 @@ This one-liner will get you the latest 32-bit binary:
     $ mget -O $(mget -q /Joyent_Dev/public/mdb_v8/latest)/mdb_v8_ia32.so
 
 
-## Design and implementation notes
-
-### Design constraints
+## Design goals
 
 An important design constraint on this tool is that it should not rely on
 assistance from the JavaScript runtime environment (i.e., V8) to debug Node.js
@@ -93,7 +91,8 @@ Environments](https://queue.acm.org/detail.cfm?id=2039361) outlines the history
 and motivation for postmortem debugging and the challenges underlying postmortem
 debugging in higher-level languages.
 
-### Implementation notes
+
+## Implementation notes
 
 We built this tool on mdb for two reasons:
 
@@ -166,33 +165,7 @@ themselves.
 
 ## Contributing
 
-Contributions welcome, but please help us review your changes (and keep code
-quality high) by following these guidelines.  **If you have any questions, feel
-free to ask.**  Don't let these guidelines be a barrier to contributing!
-
-If you're not sure exactly what change you want to make, create an issue to
-discuss it.  Once you've got a change ready to integrate, submit a pull request.
-
-**Formatting nits**: Pull requests should include text explaining the suggested
-change.  (Do not put this text in the commit message.  The commit message should
-consist of one line per logical change, each consisting of the issue number and
-synopsis.  See previous commit messages for examples.)
-
-**Completeness:** Pull requests should include relevant updates to the
-documentation, including CHANGES.md.  New commands, walkers, and non-private
-options must have associated documentation, both in the dmod (so that "::help
-DCMD" works) and in the usage guide.
-
-**Testing**: Code changes should be "make prepush" clean.  Major changes should
-be manually tested on:
-
-- core files from each of the three latest major release of Node
-  (e.g., Node v4, Node v0.12, and Node v0.10)
-- core files from both 32-bit and 64-bit programs
-- core files generated by abort(3c) and core files generated by gcore(1M)
-- an existing collection of representative core files
-
-There's a tool to help test on multiple Node versions in `tools/runtests_node`.
+See the [Developer's Notes](docs/development.md) for details.
 
 
 ## License

docs/development.md

@@ -0,0 +1,145 @@
+<!--
+    This Source Code Form is subject to the terms of the Mozilla Public
+    License, v. 2.0. If a copy of the MPL was not distributed with this
+    file, You can obtain one at http://mozilla.org/MPL/2.0/.
+-->
+
+<!--
+    Copyright (c) 2015, Joyent, Inc.
+-->
+
+# mdb_v8 Developer's Notes
+
+## Contribution guidelines
+
+Contributions welcome, but please help us review your changes (and keep code
+quality high) by following these guidelines.  **If you have any questions, feel
+free to ask.**  Don't let these guidelines be a barrier to contributing!
+
+If you're not sure exactly what change you want to make, create an issue to
+discuss it.  Once you've got a change ready to integrate, submit a pull request.
+
+**Formatting nits**: Pull requests should include text explaining the suggested
+change.  (Do not put this text in the commit message.  The commit message should
+consist of one line per logical change, each consisting of the issue number and
+synopsis.  See previous commit messages for examples.)  Before integration, the
+PR should be squashed into one commit.
+
+**Completeness:** Pull requests should include relevant updates to the
+documentation, including CHANGES.md.  New commands, walkers, and non-private
+options must have associated documentation, both in the dmod (so that "::help
+DCMD" works) and in the usage guide.
+
+**Testing**: All changes should be `make prepush` clean, but additional testing
+is probably necessary for most changes.  See below.
+
+
+## Testing
+
+The appropriate level of testing depends on the scope of the change.  Any
+non-trivial changes should be tested on:
+
+- core files from each of the latest several major releases of Node
+  (e.g., Node v4, Node v0.12, and Node v0.10)
+- core files from both 32-bit and 64-bit programs
+- core files generated by abort(3c) and core files generated by gcore(1M)
+
+Depending on the change, you may want to test it manually on an existing
+collection of representative core files.  If you're not sure about test
+requirements, please ask.
+
+
+### Automated testing
+
+The automated tests are not a comprehensive regression test suite.  Think of
+them more as a smoke test.  You can run them by running:
+
+- `make test` (which is run by `make prepush`)
+- the `catest` tool, which lets you run individual tests separately
+- any of the individual tests by hand (they're standalone programs), which is
+  likely easier for debugging the tests
+
+All of these approaches use whichever Node is on your PATH to run a Node program
+and generate core files of that program.  Your local build of mdb\_v8 is used to
+inspect it.
+
+
+### Automated testing across different Node versions and architectures
+
+For coverage across 32-bit and 64-bit and multiple Node versions, you can use
+the `runtests_node` tool:
+
+1. First, run `runtests_node setup SOME_DIRECTORY`, which downloads the
+   supported Node versions into "SOME\_DIRECTORY".
+2. Then, run `runtests_node run SOME_DIRECTORY`, which executes the full test
+   suite on each of the versions in the directory.
+
+Because we've made it pretty straightforward, we expect all changes to pass the
+test suite on all of the versions and architectures used by `runtests_node`.
+
+
+### Memory leak testing
+
+mdb runs with libumem, which means it's easy to check for memory leaks in C
+code.  The process basically looks like this:
+
+1. Open up MDB on a core file and load mdb\_v8.
+2. Exercise as much mdb\_v8 functionality as you can (or whatever functionality
+   you're trying to test for leaks.
+3. _On platforms before illumos issue
+   [6338](https://www.illumos.org/issues/6338) was fixed (which is all platforms
+   before 2015-10-26)_, run: `::unload libumem.so`.  This is the easiest way to
+   work around that issue.
+4. Use `gcore` to save a core file of your MDB process.
+5. Open up that core file in MDB.  (Yes, this gets confusing: you're using MDB
+   to look at a core file of MDB looking at a core file of a Node program.)
+6. Run `findleaks -v` to look for leaks.
+
+There's a lot more information about using libumem to debug memory leaks
+elsewhere on the web.
+
+
+### Stress testing
+
+There's a [dumpjsobjects](../tools/dumpjsobjects) tool that takes a core file
+and an mdb\_v8 binary and uses `findjsobjects` and `jsprint` to enumerate all of
+the objects in a deterministic order and print them out.  With a representative
+core file, this is a decent coverage test, since it will likely exercise the
+paths for interpreting most types of supported objects.  At the very least, this
+should not crash.
+
+
+### Comparing output across mdb_v8 changes
+
+For some kinds of changes, it's worthwhile to spend time comparing output from
+before the change with output after the change to make sure you haven't
+introduced any new issues.  The `mdbv8diff` tool can be used to compare the
+`dumpjsobjects` output from two different versions of mdb\_v8.
+
+Why not just use `diff(1)`?  The challenge is that many changes to mdb\_v8
+change the output format slightly or fix nitty bugs that affect a large number
+of objects.  `diff` can easily produce hundreds of thousands of lines of output,
+but there may be only a handful of different _kinds_ of changes, and they may
+all be easy to programmatically identify (e.g., the wording of a particular
+error message changed).  For this reason, `mdbv8diff` provides a small "ignore"
+module interface, where you define a function that takes two lines of output
+that look different and decides whether the difference corresponds exactly to an
+expected change.  Sometimes, a diff represents something that's hard to
+programmatically recognize but you've manually confirmed that it's correct.  You
+can also add specific values as special cases to ignore.  (These obviously
+depend on the specific core file you're debugging.)
+
+The intended workflow is that you run `mdbv8diff` on output from before and
+after your change.  It likely finds tons of diffs.  You create a small "ignore"
+module.  As you go through the diffs one-by-one, if the diff represents a
+regression, you fix your code.  If the diff represents an expected change, you
+teach your "ignore" module how to recognize it.  You keep iterating this process
+until `mdbv8diff` produces no more output.
+
+This is obviously pretty crude.  Think of this as a tool to assist the otherwise
+manual effort of exhaustively comparing lots of output, not an efficient
+regression tester.
+
+The "ignore" modules for past changes are committed to the repo for reference,
+though they likely wouldn't be reused except to reproduce the original testing
+work for the change.