gwtproject / gwt

GWT Open Source Project
http://www.gwtproject.org
1.51k stars 372 forks source link

Stack emulation with compiler.emulatedStack.recordLineNumbers produces incorrect results #8108

Open dankurka opened 9 years ago

dankurka commented 9 years ago

Originally reported on Google Code with ID 8112

Found in GWT Release (e.g. 2.4.0, 2.5.0 RC): 2.5.0

Detailed description (please be as specific as possible):

Some of the line numbers recorded by JsStackEmulator are incorrect.  So far, I've discovered
two independent causes for these inaccuracies:

1) Some of the line numbers come from methods inlined from other source files (therefore
the line number doesn't match the filename reported by StackTraceDeobfuscator)

2) Some of the JS expressions instrumented with line numbers are actually generated
"magic" code that had no actual counterpart in the original Java source code.  In these
cases, the recorded line numbers don't make sense.

Shortest code snippet which demonstrates issue (please indicate where
actual result differs from expected result):

The following stack trace, created with compiler.stackMode=true, compiler.emulatedStack.recordLineNumbers=true,
and StackTraceDeobfuscator, demonstrates both types of pathologies:

com.typeracer.main.client.view.menu.MainMenu$MainMenuItem.onClick(MainMenu.java:20)
com.google.gwt.event.dom.client.ClickEvent.dispatch(ClickEvent.java:55)
com.google.web.bindery.event.shared.SimpleEventBus.$doFire(SimpleEventBus.java:194)
com.google.gwt.event.shared.HandlerManager.$fireEvent(HandlerManager.java:127)
com.google.gwt.user.client.ui.Widget.$fireEvent(Widget.java:129)
com.google.gwt.event.dom.client.DomEvent.fireNativeEvent(DomEvent.java:116)
com.google.gwt.user.client.ui.Widget.$onBrowserEvent(Widget.java:557)
com.google.gwt.user.client.ui.Widget.onBrowserEvent(Widget.java:163)
com.google.gwt.user.client.DOM.dispatchEvent(DOM.java:1307)
com.google.gwt.core.client.impl.Impl.apply(Impl.java:189)
com.google.gwt.core.client.impl.Impl.entry0(Impl.java:242)

1) Widget.java:557 is an example of pathology #1: the onBrowserEvent method contains
code that was inlined from line 557 of UIObject.java.  Widget.java only has 482 lines
in total.

2) SimpleEventBus.java:194 and ClickEvent.java:55 are both examples of pathology #2:
these are lines that could not possibly have been part of a Java stack trace because
they don't contain any executable code.  Let's first examine one of those original
Java sources and then we'll take a look at the generated Javascript, which will demonstrate
the pathology.

ClickEvent.java:
55:  protected void dispatch(ClickHandler handler) {
56:    handler.onClick(this);
57:  }

Compiled JavaScript (PRETTY):

defineSeed(168, 169, {}, ClickEvent_0);
_.dispatch = function dispatch_1(handler){
  var stackIndex;
  $stack_0[stackIndex = ++$stackDepth_0] = dispatch_1;
  $location_0[stackIndex] = 56 , dynamicCast(($location_0[stackIndex] = 55 , handler),
Q$ClickHandler).onClick(this);
  $stackDepth_0 = stackIndex - 1;
}

The error is caused by instrumenting the dynamicCast function call with line number
55, which in the original Java source is just a plain parameter declaration.

Workaround if you have one:

I'm working on a patch that will address the two issues in the following ways:

1) Modify JsStackEmulator to record obfuscated filenames along with line numbers for
locations where the filename of the expression doesn't match the filename of the function
(i.e. where the expression was inlined from a function that was declared in a different
file). The extra deobfuscation info will be recorded in the symbol map file and StackTraceDeobfuscator
will be modified to use this additional information.

2) Modify JsStackEmulator to omit line number instrumentation for expressions when
the original Java line did not contain executable code that could throw an exception.

After extensive research experimentation, I believe that it is not possible to produce
perfectly accurate Java stack traces in Javascript, but I believe that these two modifications
will get us most of the way there.

Links to relevant GWT Developer Forum posts:

Link to patch posted at http://gwt-code-reviews.appspot.com:

Patch not ready yet.  I will update this ticket when I finish it.

Reported by alexander.epshteyn on 2013-04-21 20:06:53

dankurka commented 9 years ago
There's actually another pathology that I didn't mention in the original description:

3) Function invocation instrumentations don't match the execution order. 

Here's an artificial example.

Suppose G is the line number of the invocation of g, F is the line number of the invocation
of f, and the original code is

f(x, g(y))

JsStackEmulator.java from GWT 2.5 instruments that code as follows:

location[stackIndex]=F, f(x, (location[stackIndex]=G,g(y)))

This is wrong because once control passes into f, the line number in the previous stack
frame will be G, while it should really be F.

Solution:

The patch I'm working on will introduce a temporary variable along with some cleverly-situated
comma expressions that will allow recording the line number of the original invocation
at the end of the argument list, such that the above example will be rewritten as follows:

f(x,(temp=(location[stackIndex]=G,g(y)),(location[stackIndex]=F,temp)))

Reported by alexander.epshteyn on 2013-04-23 07:47:04

dankurka commented 9 years ago
Correction: the ClickEvent example actually demonstrates problem #3, not #2.

Reported by alexander.epshteyn on 2013-04-23 09:18:53

dankurka commented 9 years ago
Hi Alexander,

please make sure to discuss this on gwt contrib as well. So that other can help you
figure this one out!

Reported by dankurka@google.com on 2013-05-27 21:19:54