bugmake - Bugs: bug #23922, On XP 64 with sh.exe installed in...

 
 

bug #23922: On XP 64 with sh.exe installed in "Program Files (x86)" gmake generates a stack overflow exception.

Submitter:  None
Submitted:  Thu 24 Jul 2008 09:21:24 AM UTC
   
 
Severity:  3 - Normal Item Group:  Bug
Status:  Fixed Privacy:  Public
Assigned to:  eliz Open/Closed:  Closed
Component Version:  3.81 Operating System:  MS Windows
Fixed Release:  4.0 Triage Status:  Small Effort
* Mandatory Fields

Add a New Comment Rich Markup
   

Thu 17 Sep 2015 03:41:24 PM UTC, comment #4: 

The original problem happened because 32-bit programs on 64-bit versions of MS-Windows are routinely installed under the system directory called "C:/Program Files (x86)/".  For GNU Make to work correctly on such systems, when SHELL is simply set to the absolute file name of a shell installed under that directory, the parentheses must be quoted.  If we don't do that, GNU Make on Windows would be useless with alternate shells.

IOW, the fix in commit b5c06541 fixed a serious problem whose probability of showing up on MS-Windows is very high.

By contrast, the use case it broke is quite rare, and the way it (ab)uses the SHELL variable is quite frankly unsupported on MS-Windows -- unlike on Posix systems, the MS-Windows port of Make does not yet support arbitrary commands as the value of SHELL, and never did.  So it's a small wonder that this use case became broken.

> Whoever defines SHELL can be presumed to know what they mean by it.


There's nothing wrong with using a file name whose directories include parentheses.  Users on Windows should not be required to escape them.  We cannot ask them to; this is a non-starter.

> Surely it doesn't matter if SHELL has characters special to the shell, unless it is being invoked by the shell, in which case can we limit the escaping to the circumstance when it is actually invoked by the shell, and not by execve ?


Unfortunately, this distinction on Windows is problematic.  Even if we initially intend to bypass the shell, later we might decide we do need it, for various obscure reasons.  Also, CreateProcess, the API used to run subsidiary programs on Windows, needs its command-line arguments to observe some of the rules that are identical to those of the shell.

> If we insist on doing this, can we have an option to disable it? Or to enforce that it be invoked by the shell instead of execve?


Patches to achieve that will be welcome, if they are clean and don't affect any other use cases.  Personally, I think the latter alternative is all but impossible, but I could be wrong.

Please note that for a large enough changeset you will have to sign legal papers before the changes can be accepted.

Thanks.

Eli Zaretskii <eliz>
Group Member
Thu 10 Sep 2015 10:31:44 AM UTC, comment #3: 

This was fixed by git commit b5c06541

This change has broken my makefiles.

I think it is appropriate to extra-quote commands passed to the shell, but not those in the SHELL definition itself.

Whoever defines SHELL can be presumed to know what they mean by it.

Surely it doesn't matter if SHELL has characters special to the shell, unless it is being invoked by the shell, in which case can we limit the escaping to the circumstance when it is actually invoked by the shell, and not by execve ?

If we insist on doing this, can we have an option to disable it? Or to enforce that it be invoked by the shell instead of execve?

Detail:

Some make files, generated using a literate programming tool fangle (https://savannah.nongnu.org/p/fangle) have SHELL defined thus:

SHELL:=bash -c 'exec $(SHELL) "$${@//\\$$'\''\012'\''/$$'\''\012'\''}"' --

as you see, there are many shell-special characters there, but they are already quoted appropriately.

This fix now extra quotes characters like $ preventing their proper interpretation by the shell.

The SHELL contrivance above is to permit use of \ continuations at the end of a recipe line but preserving the \n to the shell, avoiding the need of the common but painful ; \ construct which is not valid in many embedded commands, e.g. sed invocations with complex rules as part of the make recipe.

.ONESHELL does a similar thing but without the selective control of the trailing \ (as well as other harm by difficult-to-control stripping of @ - in the middle of the recipe).


Sam Liddicott <samjam>
Sat 07 May 2011 08:43:05 AM UTC, comment #2: 

That "syntax error" seen by Paul is the symptom of the same problem: Make assumed without checking that the value of "SHELL" does not have characters special to the shell in it.

I fixed that with the attached patch.

Note that a similar change was suggested in patch #7522, but I decided to use a less radical approach, and also fixed another problem that could cause a crash.


(file #23363)

Eli Zaretskii <eliz>
Group Member
Sun 02 Aug 2009 11:48:25 PM UTC, comment #1: 

I tried to reproduce this on Linux and couldn't (I got an error /bin/sh: Syntax error: word unexpected (expecting ")") instead).  Setting this to MS-Windows.

Paul D. Smith <psmith>
Group administrator
Thu 24 Jul 2008 09:21:24 AM UTC, original submission:  

This seems to be a combination of the "()"s in the path for sh, and using output redirection in a rule. Our makefile includes the following rule:

${UNIAV_H}: mkuniav.c quadav.h
        mk_cc $<
        mk_lnk ${call getexe, $<} ${call getobj, $<}
        mkuniav.exe > ${UNIAV_H}

The redirection on the last line of the rule causes gmake to enter an infinite loop generating the command line to execute. Removing the ">" or moving sh.exe to a directory without "()"s in works around the problem.


Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #23363:  w32shellspecials.dif added by eliz (2KiB - application/vnd.ms-excel)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by samjam (Posted a comment)
  • -email is unavailable- added by eliz (Updated the item)
  • -email is unavailable- added by psmith (Posted a comment)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2011-05-07 psmith Assigned toNone eliz
    2011-05-07 eliz Attached File- Added w32shellspecials.dif, #23363
        StatusNone Fixed
        Open/ClosedOpen Closed
        Fixed ReleaseNone 4.0
        Triage StatusNone Small Effort
    2009-08-02 psmith Operating SystemNone MS Windows

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code