--debugger=lldb doesn't work with mochitests

RESOLVED FIXED in mozilla28

Status

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: heycam, Assigned: froydnj)

Tracking

unspecified
mozilla28
x86
Mac OS X
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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
Does lldb require some other way of passing cmdline arguments to the debuggee?
(Assignee)

Comment 2

5 years ago
(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.
This used to work for me until today, so presumably something regressed it.
Keywords: regressionwindow-wanted
Well, I don't know how useful a regression window will actually be. Nathan, can you look at this when you get a sec?
Flags: needinfo?(nfroyd)
Keywords: regressionwindow-wanted
Seeing this too. It's pretty weird that it cuts off the url at the first '&'.
Setting target.run-args[0] to the url with the ampersand escaped seems to avoid the problem.
(Assignee)

Comment 7

5 years ago
(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
Flags: needinfo?(nfroyd) → needinfo?(jmaher)
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?
Flags: needinfo?(jmaher)
(Assignee)

Comment 9

5 years ago
(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.
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.
(Assignee)

Comment 11

5 years ago
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.
(Assignee)

Comment 12

5 years ago
Created attachment 8341817 [details] [diff] [review]
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.
Attachment #8341817 - Flags: review?(jmaher)
Attachment #8341817 - Flags: feedback?(bobbyholley+bmo)
Comment on attachment 8341817 [details] [diff] [review]
escape URL arguments in mochitests for the benefit of LLDB

Works great for me. nfroyd++!
Attachment #8341817 - Flags: feedback?(bobbyholley+bmo) → feedback+
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?
Attachment #8341817 - Flags: review?(jmaher) → review+
(Assignee)

Comment 15

5 years ago
(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...
(Assignee)

Comment 16

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/895031613c50
Assignee: nobody → nfroyd
Flags: in-testsuite-
https://hg.mozilla.org/mozilla-central/rev/895031613c50
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
You need to log in before you can comment on or make changes to this bug.