508721 - WinCE build tools do not understand the first argument provided

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Alex Pakhotin (:alexp)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.13) Gecko/2009073022 Firefox/3.0.13 (.NET CLR 3.5.30729)
Build Identifier: 

WinCE build tool wrappers located under build\wince\tools lose the first argument passed to the actual EXEs. For example arm-wince-res.c passes an additional include directory to rc.exe, which gets lost, causing "cannot find resdefce.h" compile error.


Reproducible: Always

Steps to Reproduce:
Try to compile a .rc resource file, which includes resdefce.h.

Comment 1

[Alex Pakhotin (:alexp)]

Attachment: Suggested fix

Current implementation of the compile tools execution using CreateProcess completely loses the first argument passed to those tools. This article describes the issue: http://support.microsoft.com/kb/175986
We have the case #3 from the article running the tools this way: CreateProcess( "c:\\MyApp.exe", "Param1 Param2 Param3", ...)
In this case the arguments passed to the process are:
  argv[0] == "Param1"
  argv[1] == "Param2"
  argv[2] == "Param3"
while normally argv[0] should contain the application name, and all the arguments should start from argv[1].

The suggested fix is to put the whole command line into the second argument of CreateProcess. It seems to work properly.

This change also showed a bug in arm-wince-as.c, where the first argument for armasm.exe was wrong. This is fixed by the patch as well.

Comment 2

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 392857 [details] [diff] [review]
Suggested fix

>-  args[i++] = "-I\"" WCE_INC "\""; 
>+  args[i++] = "-I \"" WCE_INC "\""; 

why this change?  please add a comment

Comment 3

[Brian Crowder]

It would be nice to understand what argument(s) our build tools were dropping because of this bug.

Comment 4

[Alex Pakhotin (:alexp)]

The arguments dropped were usually additional include directories, which are rarely used - that's why it was not noticed before.

Comment 5

[Alex Pakhotin (:alexp)]

Attachment: Suggested fix

Added comments describing the changes.

Comment 6

[Brian Crowder]

Comment on attachment 399825 [details] [diff] [review]
Suggested fix

http://hg.mozilla.org/mozilla-central/rev/613cf52be14d

Comment 7

[Brad Lassey [:blassey] (use needinfo?)]

pushed http://hg.mozilla.org/releases/mozilla-1.9.2/rev/1c5fdb5edba4