On 12/09/19 14:00, Philippe Mathieu-Daudé wrote:
> I forgot, to have VbeShim.sh succeed, I also have this local change:
>
> -- >8 --
> diff --git a/OvmfPkg/QemuVideoDxe/VbeShim.sh
> b/OvmfPkg/QemuVideoDxe/VbeShim.sh
> index aea28be35f5c..92b345befe1f 100755
> --- a/OvmfPkg/QemuVideoDxe/VbeShim.sh
> +++ b/OvmfPkg/QemuVideoDxe/VbeShim.sh
> @@ -28,7 +28,7 @@ trap exit_handler EXIT
> #
> # Assemble the source file.
> #
> -nasm -o "$STEM".bin -- "$STEM".asm
> +nasm -fbin "$STEM".asm -o "$STEM".bin
>
> #
> # Disassemble it, in order to get a binary dump associated with the
> source.
> ---
>
> Because I had this error:
>
> $ sh -x OvmfPkg/QemuVideoDxe/VbeShim.sh
> [...]
> ++ dirname -- OvmfPkg/QemuVideoDxe/VbeShim.sh
> ++ basename -- OvmfPkg/QemuVideoDxe/VbeShim.sh .sh
> + STEM=OvmfPkg/QemuVideoDxe/VbeShim
> + trap exit_handler EXIT
> + nasm -o OvmfPkg/QemuVideoDxe/VbeShim.bin --
> OvmfPkg/QemuVideoDxe/VbeShim.asm
> nasm: error: more than one input file specified
> nasm: error: more than one input file specified
> nasm: error: more than one input file specified
> type `nasm -h' for help
> + exit_handler
>
> Which is rather confuse, because the help is still recommending the
> current form:
>
> $ nasm -h
> usage: nasm [-@ response file] [-o outfile] [-f format] [-l listfile]
> [options...] [--] filename
> or nasm -v (or --v) for version info
>
> -t assemble in SciTech TASM compatible mode
> -E (or -e) preprocess only (writes output to stdout by default)
> -a don't preprocess (assemble only)
> -M generate Makefile dependencies on stdout
> -MG d:o, missing files assumed generated
> -MF <file> set Makefile dependency file
> -MD <file> assemble and generate dependencies
> -MT <file> dependency target name
> -MQ <file> dependency target name (quoted)
> -MP emit phony target
>
> -Z<file> redirect error messages to file
> -s redirect error messages to stdout
>
> -g generate debugging information
>
> -F format select a debugging format
>
> -gformat same as -g -F format
>
> -o outfile write output to an outfile
>
> -f format select an output format
>
> -l listfile write listing to a listfile
>
> -I<path> adds a pathname to the include file path
> -O<digit> optimize branch offsets
> -O0: No optimization
> -O1: Minimal optimization
> -Ox: Multipass optimization (default)
>
> -P<file> pre-includes a file
> -D<macro>[=<value>] pre-defines a macro
> -U<macro> undefines a macro
> -X<format> specifies error reporting format (gnu or vc)
> -w+foo enables warning foo (equiv. -Wfoo)
> -w-foo disable warning foo (equiv. -Wno-foo)
>
> -w[+-]error[=foo] can be used to promote warnings to errors
> -h show invocation summary and exit
>
> --prefix,--postfix
> these options prepend or append the given string
> to all extern and global variables
>
Yeah I think this is a NASM regression. In my version of NASM
(nasm-2.10.07-7.el7.x86_64, on RHEL7), the manual says, near "-f":
> To see a list of valid output formats, use the -hf option.
And when I invoke NASM like that, I get:
> valid output formats for -f are (`*' denotes default):
> * bin flat-form binary files (e.g. DOS .COM, .SYS)
So NASM either removed any default for "-f", or changed the default. Let
me check...
At upstream commit e91f5cc1322e ("preproc: fix %undef of macro aliases,
and add %ifdefalias", 2019-10-23), the source file "output/outform.h"
still contains:
> #ifndef OF_DEFAULT
> #define OF_DEFAULT of_bin
> #endif
This can be overridden when building NASM:
> * Default config = -DOF_ALL -DOF_DEFAULT=of_bin
However, if I check Fedora dist-git at commit 0150c8afb52f ("Rebuilt for
Python 3.8", 2019-08-19) (the HEAD of the f30 branch), then I do *not*
see any OF_DEFAULT mentions.
...
NASM in Fedora 30 seems to be based on 2.14.02. Upstream tag "nasm-2.14"
contains commit 987dc9c9dbe3 ("Make any execution limit configurable,
add eval limit", 2018-06-12). This commit seems to have changed how the
"--" options-terminator is processed:
> @@ -1068,25 +1062,61 @@ static bool process_arg(char *p, char *q, int pass)
> case '-':
> {
> const struct textargs *tx;
> + size_t olen, plen;
>
> - if (p[2] == 0) { /* -- => stop processing options */
> + p += 2;
> +
> + if (!*p) { /* -- => stop processing options */
> stopoptions = true;
> break;
> }
I don't know if this is actually the problem, it just looks related. Can
you report a bug in the upstream tracker? <https://bugzilla.nasm.us/>
If NASM upstream confirms the bug, then I'd like to request that you
please submit this patch for "OvmfPkg/QemuVideoDxe/VbeShim.sh",
including a comment to the upstream NASM bug report.
To be clear, the original -- now apparently broken -- NASM command line
follows the "Utility Syntax Guidelines" from POSIX:
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html#tag_12_02
See in particular Guideline 9 ("All options should precede operands on
the command line") and Guideline 10 ("The first '--' argument that is
not an option-argument should be accepted as a delimiter indicating the
end of options"). Namely:
* '-o "$STEM".bin' is an option, taking an option-argument. Therefore it
should go first.
* The argument '--' is the end-of-options delimiter, which is generally
recommended every time an operand is substituted. (For example, if we
want to delete "$HELLO", we don't just write 'rm -rf "$HELLO"', but
"rm -rf -- "$HELLO".)
* The argument '"$STEM".asm' is an operand.
The command line that your patch adds:
- breaks Guideline 9, because it intermixes options and operands;
- breaks Guideline 6, because it contracts "-f" with "bin" into a single
argument, even though "-f" takes a mandatory -- i.e. not optional --
option-argument (such as "bin"). Per guideline 6, if an option
requires a mandatory option-argument, then the option itself, and its
argument, should be separate arguments.
I don't meant that your patch is wrong -- I mean that NASM could be
wrong to require us to use this syntax. Hence my request to file a bug
report for upstream NASM first.
Thank you!
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#52049): https://edk2.groups.io/g/devel/message/52049
Mute This Topic: https://groups.io/mt/65940640/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
