Prevent corruption of disassembler listing command for paths w/ spaces

[per1234]

This platform uses a build hook to save a disassembler listing to the build folder when compiling sketches.

Due to the use of a redirection operator in the command, it was necessary to execute it via the Windows command interpreter by passing it as an argument to cmd /c.

Although paths with spaces can be used without difficulty in pattern generated commands executed directly by the Arduino framework, things are more complicated when it comes to executing commands indirectly via cmd /c.

The cmd /c command has an odd handling of quotes:

https://learn.microsoft.com/en-us/windows-server/administration/windows-commands/cmd#remarks

Under certain conditions, the first and last quotation mark of the command passed via the argument to /c are stripped.

These conditions occur for the command generated via the platform's recipe.hooks.objcopy.postobjcopy.1.pattern.windows property when the both the path to avr-objdump and the temporary build path contain a space.

The removal of the essential quotes from the start and end of the command executed by cmd /c causes an error. for example, if the avr-objdump path was:

C:\Users\username with spaces\AppData\Local\\Arduino15\packages\arduino\tools\avr-gcc\7.3.0-atmel3.6.1-arduino7\bin\avr-objdump

The command will fail with the error:

'C:\Users\username' is not recognized as an internal or external command, operable program or batch file.

One of the required conditions for the unwanted quote stripping is that the first character of the argument to cmd /c is a double quote. So this unwanted behavior can be avoided by ensuring the first character is not a double quote. That is accomplished here by adding a dummy command (adds a blank line to the output, but the most suitable "no operation" candidate I found) that does not require quoting at the start of the argument.

Demo of bug

$ arduino-cli version
arduino-cli.exe  Version: 0.27.1 Commit: a900cfb2 Date: 2022-09-18T15:57:51Z

$ export ARDUINO_DIRECTORIES_DATA="/tmp/arduino-cli-directories/has space/data" # Cause avr-objdump to be installed under a path with spaces

$ export ARDUINO_BOARD_MANAGER_ADDITIONAL_URLS="https://mcudude.github.io/MiniCore/package_MCUdude_MiniCore_index.json"

$ arduino-cli core update-index
Downloading index: package_index.tar.bz2 downloaded
Downloading index: package_MCUdude_MiniCore_index.json downloaded

$ arduino-cli core install MiniCore:avr

[...]

$ export ARDUINO_DIRECTORIES_USER="/tmp/arduino-cli-directories/user"

$ git clone https://github.com/MCUdude/MiniCore "${ARDUINO_DIRECTORIES_USER}/hardware/MiniCore-dev"

[...]

$ arduino-cli sketch new "/tmp/FooSketch"

Sketch created in: C:\Users\per\AppData\Local\Temp\FooSketch

$ arduino-cli compile --fqbn MiniCore-dev:avr:328 --verbose --build-path "/tmp/arduino-cli-directories/has space/build" "/tmp/FooSketch"

[...]

cmd /C "C:\\Users\\per\\AppData\\Local\\Temp\\arduino-cli-directories\\has space\\data\\packages\\arduino\\tools\\avr-gcc\\7.3.0-atmel3.6.1-arduino7/bin/avr-objdump" --disassemble --source --line-numbers --demangle --section=.text "C:\\Users\\per\\AppData\\Local\\Temp\\arduino-cli-directories\\has space\\build/FooSketch.ino.elf" > "C:\\Users\\per\\AppData\\Local\\Temp\\arduino-cli-directories\\has space\\build/FooSketch.ino_atmega328p_16000000L.lst"
'C:\Users\per\AppData\Local\Temp\arduino-cli-directories\has' is not recognized as an internal or external command,
operable program or batch file.

[...]

(Arduino CLI used only because it makes it easy to customize the paths to contain spaces)

Additional information

Alternatives considered

It might seem that a less hacky approach would be to simply wrap the entire argument to cmd /c in an extra layer of sacrificial quotes, leaving the essential quotes intact in the command that is finally executed. While that does indeed work when running the command directly from the command line, it does not work when used in a pattern in the Arduino platform configuration framework. The problem is that the string generated from the platform pattern can not be executed as is. It must be split into arguments. The list of arguments space separated, with quoting where an argument contains a space.

These arguments may contain quote characters, which must be escaped. This escaping is where the simple approach of extra quotes becomes unusable. The os/exec Go package used to handle the escaping does that in a way that is compatible with directly executing a command, but is not compatible with the legacy escaping system used by cmd:

https://pkg.go.dev/os/exec#Command

On Windows, processes receive the whole command line as a single string and do their own parsing. Command combines and quotes Args into a command line string with an algorithm compatible with applications using CommandLineToArgvW (which is the most common way). Notable exceptions are msiexec.exe and cmd.exe (and thus, all batch files), which have a different unquoting algorithm. In these or other similar cases, you can do the quoting yourself and provide the full command line in SysProcAttr.CmdLine, leaving Args empty.

Scope of bug

It seems this problem of the command having paths with spaces did not occur in Arduino IDE 1.x due to it using a Windows "8.3" format for the default build path, which does not contain spaces. Arduino CLI (and thus Arduino IDE 2.x) uses the full path format instead. However, the bug could also occur in Arduino IDE 1.x if the user sets a custom build.path with spaces.

recipe.hooks.savehex.presavehex.2.pattern.windows also uses cmd /C. However, this command is not subject to the quote stripping because it does not ever have a double quote as the first character of the argument.

Related

Originally reported by @cattledogGH

Previous discussion at MCUdude/MegaCoreX#159

[MCUdude]

Thank you Per!