Re: Crash on Windows when SHELL is assigned

[Orgad Shaneh]

On Tue, Jun 6, 2017 at 8:18 PM, Eli Zaretskii <address@hidden> wrote:
>> From: Orgad Shaneh <address@hidden>
>> Date: Mon, 5 Jun 2017 23:13:26 +0300
>>
>> 1 You must use a full name for SHELL, including the extension (SHELL=sh 
>> doesn't work. SHELL=sh.exe
>>  does, if sh.exe exists in PATH). PATHEXT should be used for the executable 
>> detection.
>
> Can you tell why this is a problem?  There's no equivalent of PATHEXT
> on Posix systems, and no one coded anything like that for Windows.  Is
> it really such a serious problem?  If so, in what use cases?

Ah, the PATH lookup I referred to is in #ifdef WINDOWS32.

I have a script that calls make with SHELL=sh, and I don't want to
adapt it (and make it sh.exe) specifically for Windows.

>> 2 It looks like if PATH contains quotes, they're treated in a strange 
>> manner. Instead of just removing
>>  (unescaping) them, make looks for <run directory>\"some directory in 
>> quotes" (observed with Process
>>  Monitor). The quotes must be removed before calling _access().
>
> An example would help, but generally speaking PATH with quotes is
> invalid; see for example this post:
>
>   
> https://serverfault.com/questions/349179/path-variable-and-quotation-marks-windows
>
> If we accept that such values of PATH is invalid, then what you
> describe hits "undefined behavior", and Make is not at fault.  If you
> disagree, please describe use cases where it is hard or impossible to
> fix PATH so that it has no quotes.

I accept it. I just noticed it when I did some experiments to
workaround the 1st issue :)

>> 3 If the shell is not found, make crashes. do_variable_definition() reaches 
>> lookup_variable(), which returns
>>  NULL. Then the NULL v is dereferenced (v->append = append etc.)
>>  I tried adding NULL validations. This prevents the crash, but then SHELL=sh 
>> becomes the target instead of
>>  declaring a variable (*** No rule to make target 'SHELL=sh'. Stop.)
>
> I cannot reproduce this in simple Makefiles I tried.  The most severe
> problem I saw was this:
>
>   D:\usr\eli\data>make -f-
>   SHELL=sh.exe
>
>   all:
>           $(SHELL) -c echo foo
>   ^Z
>   sh.exe -c echo foo
>   process_begin: CreateProcess(NULL, sh.exe -c echo foo, ...) failed.
>   make (e=2): The system cannot find the file specified.
>   make: *** [D:/usr/tmp/Gma08172:4: all] Error 2
>
> which doesn't fit your description.  Can you show an example where
> Make crashes due to NULL pointer dereference?

Sorry, I forgot to mention that I refer to command-line argument (make
SHELL=sh. You can replace sh with anything that doesn't exist like
make SHELL=fsdfsdf. SHELL=cmd is an exception, since it is handled
separately, and BTW it does support cmd and cmd.exe). You don't even
need a Makefile to reproduce the crash.

> And one more thing: you didn't say which version of Make did you use
> in your testing, and where/how did you obtain its binary; I used the
> latest version 4.2.1.  Should you wish to try the version I use, you
> can find its precompiled binaries here:

I used 4.2.1 from MSYS2/mingw32, and built it from source for
debugging (after I experienced the crash).

- Orgad