943734 - --debugger=lldb doesn't work with mochitests

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Cameron McCormack (:heycam)]

I think this is a different issue from bug 940930.  There is something wrong with the arguments passed to the debuggee.

$ ./mach mochitest-plain --debugger=lldb layout/style/test/test_pseudoelement_state.html
...
(lldb) settings show target.run-args
target.run-args (array of strings) =
  [0]: "http://mochi.test:8888/tests/layout/style/test/test_pseudoelement_state.html?autorun=1&closeWhenDone=1&consoleLevel=INFO&failureFile=/z/moz/b/obj-mac-dbg/.mozbuild/mochitest_failures.json&dumpOutputDirectory=%2Fvar%2Ffolders%2F04%2Frtvfhstx5c1_y4sv_179m40c0000gn%2FT"
  [1]: "-foreground"
  [2]: "-profile"
  [3]: "/var/folders/04/rtvfhstx5c1_y4sv_179m40c0000gn/T/tmpyhoI5k"
$ ps axwww | grep /lldb | grep -v grep
27611 s007  S+     0:00.41 /Applications/Xcode.app/Contents/Developer/usr/bin/lldb -- /z/moz/b/obj-mac-dbg/dist/NightlyDebug.app/Contents/MacOS/firefox http://mochi.test:8888/tests/layout/style/test/test_pseudoelement_state.html?autorun=1&closeWhenDone=1&consoleLevel=INFO&failureFile=/z/moz/b/obj-mac-dbg/.mozbuild/mochitest_failures.json&dumpOutputDirectory=%2Fvar%2Ffolders%2F04%2Frtvfhstx5c1_y4sv_179m40c0000gn%2FT -foreground -profile /var/folders/04/rtvfhstx5c1_y4sv_179m40c0000gn/T/tmpf2S25D
(lldb) r
/bin/bash: -foreground: command not found
### XPCOM_MEM_BLOAT_LOG defined -- logging bloat/leaks to /var/folders/04/rtvfhstx5c1_y4sv_179m40c0000gn/T/tmpyhoI5k/runtests_leaks.log
fcntl(F_SETLK) failed. errno = 35
Process 27737 launched: '/z/moz/b/obj-mac-dbg/dist/NightlyDebug.app/Contents/MacOS/firefox' (x86_64)
error: initial process state wasn't stopped: exited
(lldb) fcntl(F_SETLK) failed. errno = 35
fcntl(F_SETLK) failed. errno = 35
fcntl(F_SETLK) failed. errno = 35
fcntl(F_SETLK) failed. errno = 35
fcntl(F_SETLK) failed. errno = 35
...repeated a good number of times...

and finally Firefox starts, but seemingly without its command line arguments, as I get the "Nightly is already running" message due to my regular use Firefox that's running:

$ ps axwww | grep firefox.*mochi | grep -v grep | grep -v lldb
27788 s007  S      0:01.66 /z/moz/b/obj-mac-dbg/dist/NightlyDebug.app/Contents/MacOS/firefox http://mochi.test:8888/tests/layout/style/test/test_pseudoelement_state.html?autorun=1

Comment 1

[(not currently active) Ted Mielczarek]

Does lldb require some other way of passing cmdline arguments to the debuggee?

Comment 2

[Nathan Froyd [:froydnj]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #1)
> Does lldb require some other way of passing cmdline arguments to the
> debuggee?

The |-- [args]| way is correct...at least it was when I added lldb support.

Comment 3

[Bobby Holley (:bholley)]

This used to work for me until today, so presumably something regressed it.

Comment 4

[Bobby Holley (:bholley)]

Well, I don't know how useful a regression window will actually be. Nathan, can you look at this when you get a sec?

Comment 5

[Peter Van der Beken [:peterv]]

Seeing this too. It's pretty weird that it cuts off the url at the first '&'.

Comment 6

[Peter Van der Beken [:peterv]]

Setting target.run-args[0] to the url with the ampersand escaped seems to avoid the problem.

Comment 7

[Nathan Froyd [:froydnj]]

(In reply to Bobby Holley (:bholley) from comment #3)
> This used to work for me until today, so presumably something regressed it.

(In reply to Peter Van der Beken [:peterv] from comment #6)
> Setting target.run-args[0] to the url with the ampersand escaped seems to
> avoid the problem.

Joel, did we change anything wrt mozrunner &co recently that might have broken exec'ing things with &-containing arguments?  I see that we use mozrunner to start mochitests:

http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py#868

and pass in debugger information:

http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py#884

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

I am not aware of any mozbase (mozprocess/mozrunner/mozprofile) changes that have taken place in the last couple weeks.  Could this have been broken for a longer period of time?

Comment 9

[Nathan Froyd [:froydnj]]

(In reply to Joel Maher (:jmaher) from comment #8)
> I am not aware of any mozbase (mozprocess/mozrunner/mozprofile) changes that
> have taken place in the last couple weeks.  Could this have been broken for
> a longer period of time?

Apparently not, according to comment 3.  I don't see any changes in testing/mochitest/ that look like they would affect this, and I see the same sort of behavior on an older copy of lldb (XCode 4.x something, I think) that I have.  So I am a little bewildered how this ever could have worked.

Comment 10

[Bobby Holley (:bholley)]

Maybe I'm totally crazy. My certainty that this used to work is around 90%.

Regardless, it probably makes sense just to figure out what's going wrong and fix it. Looks like peter has some ideas in comment 6.

Comment 11

[Nathan Froyd [:froydnj]]

Peter has the right idea, but it's not our responsibility to take care of that.  gdb, for instance, DTRT when given an argument with a |&| character in it.

lldb even has some code for escaping characters before they get passed to the shell.  They just don't handle all the characters one would need to.  (Only quotes and spaces, AFAICS, in the current sources.)  Fixing it in LLDB is pretty easy.

...but that doesn't fix this in the short term.  And any fix we make locally will eventually collide with the fix that LLDB has.

I guess we could just assume that once an XCode with a fix for this issue comes out, that everybody ought to upgrade and remove the fix from our tree.  I'll try putting together a patch.

Comment 12

[Nathan Froyd [:froydnj]]

Attachment: escape URL arguments in mochitests for the benefit of LLDB

I'm not fond of this patch, but it does seem to fix the issue for me.

Comment 13

[Bobby Holley (:bholley)]

Comment on attachment 8341817 [details] [diff] [review]
escape URL arguments in mochitests for the benefit of LLDB

Works great for me. nfroyd++!

Comment 14

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8341817 [details] [diff] [review]
escape URL arguments in mochitests for the benefit of LLDB

Review of attachment 8341817 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/mochitest/runtests.py
@@ +835,5 @@
>      # https://bugzilla.mozilla.org/show_bug.cgi?id=916512
>      args.append('-foreground')
>      if testUrl:
> +      if debuggerInfo and debuggerInfo['requiresEscapedArgs']:
> +        testUrl = testUrl.replace("&", "\\&")

is this the only thing that requires escaping?

Comment 15

[Nathan Froyd [:froydnj]]

(In reply to Joel Maher (:jmaher) from comment #14)
> Comment on attachment 8341817 [details] [diff] [review]
> escape URL arguments in mochitests for the benefit of LLDB
> 
> Review of attachment 8341817 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: testing/mochitest/runtests.py
> @@ +835,5 @@
> >      # https://bugzilla.mozilla.org/show_bug.cgi?id=916512
> >      args.append('-foreground')
> >      if testUrl:
> > +      if debuggerInfo and debuggerInfo['requiresEscapedArgs']:
> > +        testUrl = testUrl.replace("&", "\\&")
> 
> is this the only thing that requires escaping?

Technically, no.  We could escape <, >, $, and maybe some other bits of punctuation that I have forgotten.  But escaping & does the trick for now and I don't expect the other characters to show up in mochitest URLs...

Comment 16

[Nathan Froyd [:froydnj]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/895031613c50

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/895031613c50