Another 'fun' bug

Time for another bug story - this one was fun because it is a combination of programmer (me) error and a compiler error (specifically a PGO-only error).

The bug in question is bug 959842 (in particular, the second patch).

The thing that made this bug 'fun' was that it only showed up in PGO builds. PGO is profile guided optimisation, but is often used as a shorthand for both that and link time optimisation (LTO). PGO is a further optimisation pass that occurs at link time where the optimiser has global knowledge of the program (and profiles of the running program) so can do sophisticated optimisations without having to worry about 'open world' constraints. PGO is a pain because it take a long time and can be buggy (usually in the manner of a build failing at the PGO stage when it builds fine otherwise, but not this time). However, it gets us significant speed up, so Firefox releases are always built using PGO. Try builds do not use PGO and only occasional pushes to inbound do (I think). I have not got PGO builds running locally. That made this bug difficult to debug, because testing required throwing a build at try (you can change the mozconfig to get a PGO build from try), waiting for five hours, downloading the build and seeing if it crashed. Furthermore, its an optimised build so debugging using a debugger is a real pain.

The bug was also fun because it was (in part) due to me trying to be too clever with my code and falling between the cracks of C++ semantics.

So, we have an object RotatedBuffer which holds on to a DrawTarget. That DrawTarget can be loaned out to other objects so they can draw into it. However, it is important that only one other object has the DrawTarget at a time (because depending on how the DrawTarget will be used, the RotatedBuffer will set a transform on it). To handle this we have methods BorrowDrawTargetFor... and ReturnDrawTarget. The former returns a DrawTarget, the latter is used when the client object is done drawing. It looks like this:

void
BorrowDrawTarget::ReturnDrawTarget(gfx::DrawTarget*& aReturned)
{
  MOZ_ASSERT(aReturned == mLoanedDrawTarget);
  mLoanedDrawTarget->SetTransform(mLoanedTransform);
  mLoanedDrawTarget = nullptr;
  aReturned = nullptr;
}
The method takes a reference to a pointer to a DrawTarget. The last statement sets the referred to pointer to null, so that the client can no longer use the DrawTarget. That is me trying to be too clever.

The problem happens because due to an inheritance tangle we need a sub-class to have a wrapper method which statically dispatches to the base class (BorrowDrawTarget, which is a mixin class which implements the 'loan out a DrawTarget' behaviour). There are a few of these wrappers, which look like:

virtual void ReturnDrawTarget(gfx::DrawTarget* aReturned) MOZ_OVERRIDE
{
  BorrowDrawTarget::ReturnDrawTarget(aReturned);
}

(Now renamed to ReturnDrawTargetToBuffer, but that is not important).

The programmer-error part of the bug was missing the '&' after 'DrawTarget*' in the signature of these wrapper methods. That means that the caller's pointer is copied to the wrapper method, which passes a reference to its argument to BorrowDrawTarget::ReturnDrawTarget. So it is the wrapper's copy of the pointer which is set to null and the clients copy remains in existence, potentially to be (ab-)used. Unfortunately this is all valid C++ so the compiler gives us no warning that something is amiss. Indeed, running Firefox like this works just fine (since the client never actually reuses its returned pointer).

Until that is, PGO works its magic. Then (I assume, I couldn't actually make sense of the disassembly), the wrapper function is inlined and so the wrapper's copy of the pointer no longer exists. Something gets set to null by BorrowDrawTarget::ReturnDrawTarget, but I have no idea what, it certainly isn't the client's pointer. It causes a segfault and Firefox crashes. This part is a compiler error, specifically an error in the PGO(/LTO) optisation pass. That is because optimisation should not change the observable behaviour of code and this optimisation clearly does.

So, the combination of a programmer trying to be too clever, a typo, and a PGO bug caused a crash which was more than a little bit frustrating to track down, but now (hopefully) it is fixed.

A rough guide to DXR

DXR is a web-based cross-reference tool created by Mozilla. It allows for code search, navigation, and comprehension. It is a semantic tool (rather than syntactic, such as grep or MXR) and uses a compiler (Clang) plugin and static analysis to create a 'smart' index of a code base. I find it extremely useful and have contributed some bits and pieces of code, and would like to share it's wonderfulness, so here goes. DXR can be used on any code base, I mostly use the Firefox index hosted at http://dxr.mozilla.org.

DXR is being actively developed and so things in this guide might quickly become out of date. In particular, there is a large UI refresh coming up which changes a lot of the interaction modes. Another big change coming up is support for multiple trees - for now DXR can only index one tree at a time. E.g., the Firefox index only covers mozilla-central. Soon it will cover other branches and other projects too. For more information on what is coming, see https://wiki.mozilla.org/DXR_UI_Refresh and https://wiki.mozilla.org/DXR_Query_Language_Refresh.

DXR is open source and we are always keen to get contributions. Code, tests, ideas, and especially feedback are all appreciated. If you find bugs, or have ideas for improvements please file a bug in bugzilla (product=Webtools, component=DXR). You could also use the dev.static-analysis mailing list (although it is often pretty quiet) or the #static channel on irc (feel free to ping me (nrc) if you like).



I've highlighted terms I think people might want to find quickly ('how to's). Apologies if this makes for slightly obnoxious reading.

Searching and browsing

From the front page of DXR you can either search or browse. Searching will jump to the results as you type. The search functionality is the same everywhere in DXR. Browsing lets you navigate the directories and files in the repo.

Searching is case insensitive by default. There is a checkbox to the right of the box to make searching case sensitive. The search string must be at least three characters long to get results. Searching currently 'OR's together search terms (this should change soon though), so if you want to search for more than one word in a phrase you should use double quotes. The only way (currently) to search for quote literals is to use a regex and escape them there. You do a regex search using the syntax  regexp:#exp#, where exp is the regex to search for.

When using the search box you can just wait for your results to appear or you can hit 'enter'. When you hit 'enter' you ask DXR to do a 'smart' search. Without hitting 'enter' you get a plain text search. The smart search will take you to the definition of an identifier, if an identifier can be found uniquely from the search string. It will also jump to a file if your search string matches a unique file name. You can use filename.ext:n syntax to jump straight to line n in file filename.ext. In any of these cases, if you wanted a plain text search (which you get without pressing 'enter') there is a link at the top of the result page.

On the top right of the search results page (only after hitting enter, and only if there are multiple results) is the advanced search form. Here you can restrict your search to a category of identifier (type name, function name, etc.). You can also filter your search by path (which can be used to search in a given file; just a filename is ok, you don't need a complete path) and file extension. These are still text searches, just restricted, so you don't need to search for the full function name (say) or worry about case. Any of the advanced search filters can be used 'manually' in the search box at any time.

The search results page displays the results of your search ordered by file and by line within the file. Click any line to jump to that line in the file, or click the filename to jump to the top of the file. You can click any directory to get a menu too - you can then restrict the search to that directory, or exclude that directory from searches (including sub-directories in both cases).

DXR indexing

DXR does not index every file in a repo. The files it indexes get the advanced menus and highlighting of identifiers, etc. Files which are not indexed just get syntax highlighting. Importantly, only indexed pages are included in semantic searches. So, if you search for callers of a method (for example) only callers in indexed files will be shown. Plain text search covers all files.

Only C/C++ files are indexed (Rust indexing coming soon). Furthermore, only files which are touched during a DXR build are indexed. DXR currently builds on Linux and is a debug build. So code which is specific to a non-Linux platform (e.g., Windows, FirefoxOS), or code which is non-debug only is not indexed. The long and short of this is that you need to be aware that if you are relying on semantic search, then search results are only exhaustive with respect to one platform and configuration.

Code

Once on a file's page, for non-indexed files you just get a syntax highlighted copy of the file and you can click identifiers to search for that text string (via a menu). For indexed files, things are more interesting. If you click an identifier you get a menu full of options, the options depend on what kind of identifier it is. Usually you get to jump to the definition, find declarations, find references (uses of a variable or type), or search (which just does a plain text search). For methods and functions you can find all callers or all callees of the method/function. You can jump to a file from an include statement, find sub- and super- (base) classes for classes, jump to bugzilla bug references from comments, and other fun stuff.

When you have an identifier selected (and can see its menu) it is highlighted, and so are all other uses of that item in the same file (this is convenient for finding uses of a variable in a method, for example). If you hover over const variables you can see their value.

From a code page, you can always search again from the search box at the top, or go to the file's enclosing directories via the breadcrumb links at the top right.

There is no (easily accessible) function to search within the current page, but I find the browser search function is often handy for this.

On the right hand side of the page is the navigation bar. At the top are links to version control pages for that file (log, blame, etc.). Below are links to items declared on that page.

The left hand side of the page contains line numbers. You can click on one to get the URL for that line number on the current page. To the left of the line numbers you occasionally see warning icons which when clicked (or hovered, maybe) tell you about compiler warnings. These seem to be very rare, I don't know why more don't show up.

Running DXR locally

You can run DXR locally on m-c or the repo of your choice. You just need to pull the code from github, and follow the instructions in the readme file. I highly recommend the Vagrant approach. It is not trivial, but fairly easy. If you get stuck, just ask on #static. To index a repo you will need a dxr.config file for the repo, here are my dxr.configs for m-c and Rust (the latter doesn't index, just works for text search). Be aware that indexing is slow - m-c takes about four hours on a fast laptop.

Any questions?

Leave them in the comments or ping me on irc. I'm keen to help everyone get the most from this great tool.

OMTC for Windows nightly users

I just landed a patch flipping the switch for all Windows users with HWA (d3d9/10/11) to use off-main-thread compositing. This is a fairly big change to our rendering pipeline, so if you notice rendering issues on Windows, please file bugs.

For now, only nightly users will get this change. Riding the trains depends on bugs 913503 and 904890, and general stability. We wanted to land this early to get some extra testing time and because without being tested, it has been rotting super quickly. I will arrange for a branch to keep testing main thread composition asap.

One known issue is windowed plugins lagging during scrolling (913503), so please ignore that (for now) if you observe it.

OMTC can be disabled by setting the 'layers.offmainthreadcomposition.enabled' pref to false. If there are more problems than we can fix relatively quickly, we can do this for all users very easily.

This is the culmination of months of work, so it is really exciting to see it finally land. I fully expect, however, to have to turn it off due to a tidal wave of unforeseen bugs, but such is life.

No more main-thread OpenGL in Firefox (important note for Linux users who use OpenGL)

Main-thread compositing with OpenGL is no more (bug 924403). The only option for OpenGL compositing is off-main-thread compositing (OMTC). This is the first big chunk of code removal following on from the layers refactoring, which began more than a year ago. It is very nice to finally be removing code and reducing the number of code paths in the graphics module.

Most users should not notice a difference. All supported configurations which use OpenGL (FirefoxOS, Android, modern OSX) already use OMTC. If you use OpenGL on Linux, however, read on.

OpenGL in Linux

OpenGL on Linux is not a supported configuration (i.e., fixing bugs is not a priority - we would love some volunteer help here, by the way, get in contact if you're keen). However, if you have good luck with your drivers, then it works pretty well and can be enabled by setting the 'layers.acceleration.force-enabled' pref to true. The main benefit is improved WebGL performance. OMTC on Linux also works pretty well, but is also not a supported configuration. If you want to continue using OpenGL on Linux for versions of Firefox 28 and later you will need to use OMTC. (Note that if you are considering trying OpenGL on Linux for the first time, you should use a new profile to make it easier to undo if your drivers are not cooperative. And be prepared for your system to crash, potentially).

Nightly users will automatically get OMTC, if they currently get OpenGL. That is, for Nightly users, setting 'layers.acceleration.force-enabled' on Linux will get you OpenGL with OMTC.

For Aurora, Beta, and Release users (once version 28 hits those channels), you will also need to set the environment variable 'MOZ_USE_OMTC' and the 'layers.offmainthreadcomposition.enabled' pref (as well as the 'layers.acceleration.force-enabled' pref) if you want OMTC. Otherwise, you will get basic layers (software composition, although usually hardware accelerated by X).

Iterative Development

Modern software engineering 'theory' is all about iterative. Each new development seems to be more iterative than the last (contrast agile with the waterfall process, for example). It is widely acknowledged that iterative development is better than 'big bang' development.

Open source development (by which I mean the open source development 'process', as opposed to just developing software under an open source licence) is intrinsically iterative - we have a huge lump of code which we add to (or subtract from) one small (hopefully) patch at a time.

I believe that making our development of each piece (bug, issue, patch, project, whatever) iterative rather than 'big bang' is important, but difficult. One attribute of the engineers I admire at Mozilla is that they achieve this. I try, but often fail, to do so. I think it is important to be iterative because it makes it easier to catch bad design decisions earlier, makes estimation and progress assessment easier (and thus it is easier to keep interested parties happy), and, I think (and somewhat counter-intuitively), it actually results in better design in the long run. That last one is (in my fuzzy beginnings of a theory) because the developer needs a better understanding of the problem upfront to be able to identify the iterative steps required. And, as you take these steps, that understanding (and thus the solution, including earlier stages) improves.

I don't really know how to be more iterative (I'd love to hear ideas in the comments). But, I do know you have to be pretty aggressive (intellectually) in sticking to the iterative path - saving things up and landing in a big bang is often the path of least resistance.

Paris work week

I just returned from another work week, this time in Paris. Coming after the summit and some personal travel it has been a particularly exhausting month. But, as always, the work week was great. It is amazing to be able to work with such an awesome group of friendly, dedicated, cooperative, and inspirationally smart people (except gw280, he's awful). I am very lucky to work at Mozilla and with my team - being paid to work on such interesting and high impact problems with such good people is a real privilege.

Syntax IS important

In the domains of programming language theory and design I often see it stated that 'syntax is not important'. Syntax is believed to be superficial, well understood, and easily changed. As opposed to the semantics of a language which are interesting and fundamental. That is fine in the world of programming language theory (I have made similar statements myself). However, I occasionally see the same statement being made about programming language design; usually by PLT folks. In that circumstance, I believe it is wrong.

Syntax may be theoretically dull, but it is what is staring the programmer in the face every second they are using a language. The subtlest tweaks can have profound changes on how easy a language is to read and write. Language users get all excited and argumentative about tiny elements of syntax because it is important. Not because they are cretins. Syntax is the PL equivalent of a library's API. It doesn't matter how great a library is, if its API sucks, the library sucks. Likewise with PLs, it might have the greatest semantics in the world, but if the syntax sucks, then it will not be nice to use and will not get uptake (unless there is some seriously motivational use case).

Of course what makes a good syntax is subjective, and highly dependent on what syntaxes an individual is familiar with. But that makes designing a good syntax more difficult, not less important.

Summit 2013

Last weekend was the Mozilla summit. I attended the Toronto event. It was awesome. Also: exhausting, fun, informative, interesting, inspiring, sometimes awkward, sometimes over the top. A big thank you is due to the organisers. They did a fantastic job on an event of this size, everything was well planned and went smoothly and there was a nice balance of talks, free time, etc. That is really hard to get right, so kudos.

It was great to meet with lots of Mozillians outside the rendering team. That was probably the best thing about the time. Many of the technical sessions were really interesting too.

Now I just have to get over the jet lag and exhaustion from all the socialising (being sociable is hard) and get stuck into coding again.

A real-life Heisenbug

I spent three or four days debugging (a while ago now, this blog post has been aging for longer than expected in my blog cellar) an interesting and hard to find bug on Windows. It became apparent due to intermittent failures on Try with Windows OMTC builds. Finding the route cause has been an adventure and turned out to have nothing much to do with compositing or graphics. All the action is in bug 896896, if you are interested to see some code.

A Heisenbug is a bug whose behaviour is changed by observation. In this case it would go away if I set more than a few breakpoints (breakpoints which print something and continue, not actually breaking, which also caused the bug to disappear). That made debugging annoying, I was reduced to using printfs for logging, and even that sometimes affected the frequency of reproduction.

The failure occurred for mochitests with a drop down select box. About one in five test runs was timing out and thus failing. They timed out with a crash, but there was no useful crash information. There were a few mochitests which failed and the thing they had in common was a drop down list box. The first step was to try and reproduce this locally. For the longest time I could not. I tried using test pages with list boxes, all worked fine. I tried to run the mochi tests in question and it always succeeded. The simplest of the tests simulated clicking on the select box to open and close it. By accident I noticed that sometimes, the mochitest would freeze, with the list box dropped down, until I moved the mouse. A bit more experimentation showed that if the cursor was outside the window then I could move it and it would remain frozen. A bit more experimentation and I found this was specific to OMTC (it never happened with main-thread composition) and didn't depend on the actual compositor (d3d9 and d3d11 had the same problem). So, it seemed to be a problem with simulated clicks on select boxes. I tried attaching a debugger and breaking when the mochitest froze. This didn't reveal anything interesting - the compositor thread was waiting for messages and the main thread was somewhere doing Windows message stuff or sometimes GC (which is where it usually is if you randomly pause Firefox).

I then spent a day or so figuring out exactly how those simulated clicks worked. The mochitest did four clicks, and usually we got stuck between the 3rd and 4th. The test set a 500ms timeout in JS and we never got to the forth click which should happen when the timeout timed out. It took me a while to figure this out, of course. At first it looked like a problem in the code which rolled up the select box, in part because we hit a different code path for that depending on whether there is a real click or a simulated click.

So the question now looked like why wasn't our timeout completing? More logging confirmed that it was being set correctly and (eventually, in the freezing case) that it correctly called back. Investigating with Spy++ showed that when we froze, we processed a WM_PAINT message and then just stopped. So why was our message queue empty? Well if the mouse isn't in the window and nothing much else is happening, then it should be empty, and Firefox should just wait until it gets a new message. Which is what was happening, and explained why it woke up with mouse movement. But, why weren't we waking up for the timeout?

Firefox waits for a message by calling the windows function WaitMessage. That puts our thread to sleep until a message appears in our queue. We were frozen here, waiting for a message to come in. Unfortunately there are other ways to check the queue. You can call PeekMessage which reads a message from the queue and either pops it off or leaves it in place. The sad thing is that if it leaves it in place, it still counts as 'read'. The contract for WaitMessage is a bit subtle - if there is anything in the queue that is unread, it returns immediately. If the queue is empty or (and this is the nasty bit) contains only read messages, it sleeps until another message comes in. So, you must call PeekMessage just before you call WaitMessage to ensure that the queue is empty.

The bug here was that another thread (the compositor thread) was checking PeekMessage. We couldn't avoid doing this, the call comes from deep in the thread management code. We freeze when, in our main thread message loop, we check that the queue is empty and it is, then we context switched to the compositor thread which also checks the message queue, but at some point our wake up call has arrived. That gets checked by the compositor thread's event loop and is ignored and left on the queue in the 'read' state. When we context switch back to the main thread, we call WaitMessage and there are no messages so we sleep forever. It might seem unlikely that a message arrives and is checked between the queue being checked and the WaitMessage call. Calling PeekMessage doesn't just tell you if any messages have been posted to the queue, it handles any sent messages (Windows has posted (async) and sent (sync) messages). That means that during the PeekMessage call a whole lot of unknown stuff can happen. I'm a bit hazy about exactly how this bit worked out, possibly we got a paint message and that caused us to do a sync composite which context switched to the compositor thread which checked its message loop and then PeekMessaged the wake up call. In any case, this happened frequently enough to cause intermittent failure. It was a Heisenbug because by adding debugger overhead we got the wake up message entirely after the composite (I'm guessing here, btw) so I saw the bug less often or not at all.

The fix was also kind of interesting, but in a knowing the details of the Windows APIs, rather than solving a mystery kind of way. It turns out you can call WaitForMultipleObjectsEx in such a way as to return even if there are only read messages in the queue, and then sidestep the whole business.

Tip of the hat to roc and bsmedberg who helped a lot with the diagnosis and cure of this problem. I would have been stuck without their Windows knowledge and Firefox-workings-insight.

An ode to Sublime Text

I use Sublime Text as my editor for about 80% of the work I do (sometimes on Windows I use Visual Studio because I use it for debugging and the integration with the debugger is convenient. Even so, I often use Sublime Text even then because it is so much nicer). Sublime Text is pretty much a perfect piece of software. It is not often I get that warm glow from using software that does exactly what I want it to, but Sublime is one time. Here are some good things about it:

• It does one thing and it does it well (editing text for programmers).
• It is super fast and super stable - I've never had it jank (actually when searching through an 80MB file, but that is understandable) and never had it crash.
• It is beautiful - seriously, it looks really nice. That is important to me if I am going to spend most of my day looking at it.
• It is multi-platform (but not, unfortunately, open source (so not quite perfect) so I can't use it on Linux/ARM).
• It is extremely customisable. I have not found anything I want to change and can't.
• It is easy to set up your customisations across platforms because they are stored in a plain text file.
• It is smart (fuzzy search, etc.).
• It has keyboard shortcuts for everything and they are all the ones you would expect (except maybe ctrl+t which I often hit trying to open a new tab, but hey, that is customisable too).
• It's extensible with plugins.

Notice that none of the above are about editing text. Editing text is really nice too. But Sublime Text does all the 'meta' level things perfectly. That is necessary (but not sufficient) for a great piece of software. Of course you have to get the primary function right (and Sublime Text does), but so many pieces of software do their core function nicely but fail on the meta-level stuff.

Anyhow, if you haven't tried Sublime Text, you should. And if you are making software, you should strive to make it as nice as Sublime Text is.