From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.120]) by mx.groups.io with SMTP id smtpd.web09.12932.1575907935955311443 for ; Mon, 09 Dec 2019 08:12:16 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=CBgp0v1G; spf=pass (domain: redhat.com, ip: 205.139.110.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1575907935; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=PLUaB03oZLWGNN/j8TtIAfhPU1k9j6dVlF6rGC/4V7o=; b=CBgp0v1GHD/XCi4pX+VQn9718Y81Lg5mre7K8JEgdCpEwaEzFYfyyLsLs7dSYrCimS3z1m clwzJbzLXrd2e+TwFmCzJOH+Cqb5kOqzJMM6C/CoME8ffXub5Cpvf9lun38q9EaP6EXm3S LtJKBfxzCD56MHgE4+EbXK3G4ncjEsA= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-154-wNASTuJZNJe_8BMoKFQmfQ-1; Mon, 09 Dec 2019 11:12:13 -0500 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id DF113593A0; Mon, 9 Dec 2019 16:12:11 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (unknown [10.36.118.92]) by smtp.corp.redhat.com (Postfix) with ESMTP id 830625C240; Mon, 9 Dec 2019 16:12:04 +0000 (UTC) Subject: Re: [PATCH 58/79] OvmfPkg/Qemu: Fix various typos To: =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= , devel@edk2.groups.io Cc: Antoine Coeur , Jordan Justen , Ard Biesheuvel , Gerd Hoffmann References: <20191203161602.15969-1-philmd@redhat.com> <20191203161602.15969-59-philmd@redhat.com> <45bfbd81-3c44-048f-327e-3c8c3fe3009c@redhat.com> <516728a5-c9db-cb14-dd6b-2f74d0166b67@redhat.com> <391a6f7b-ba08-eb43-a6b6-2615437435c2@redhat.com> From: "Laszlo Ersek" Message-ID: <4dab8f85-5ef3-dc72-44de-eeda7778119b@redhat.com> Date: Mon, 9 Dec 2019 17:12:03 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <391a6f7b-ba08-eb43-a6b6-2615437435c2@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-MC-Unique: wNASTuJZNJe_8BMoKFQmfQ-1 X-Mimecast-Spam-Score: 0 Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 12/09/19 14:00, Philippe Mathieu-Daud=C3=A9 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=3DOvmfPkg/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 set Makefile dependency file > -MD assemble and generate dependencies > -MT dependency target name > -MQ dependency target name (quoted) > -MP emit phony target > > -Z 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 adds a pathname to the include file path > -O optimize branch offsets > -O0: No optimization > -O1: Minimal optimization > -Ox: Multipass optimization (default) > > -P pre-includes a file > -D[=3D] pre-defines a macro > -U undefines a macro > -X specifies error reporting format (gnu or vc) > -w+foo enables warning foo (equiv. -Wfoo) > -w-foo disable warning foo (equiv. -Wno-foo) > > -w[+-]error[=3Dfoo] 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 =3D -DOF_ALL -DOF_DEFAULT=3Dof_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 pas= s) > case '-': > { > const struct textargs *tx; > + size_t olen, plen; > > - if (p[2] =3D=3D 0) { /* -- =3D> stop processing o= ptions */ > + p +=3D 2; > + > + if (!*p) { /* -- =3D> stop processing options */ > stopoptions =3D 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? 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