| Message ID | 20200810130618.16066-2-r.bolshakov@yadro.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for generic ELF cross-compiler | expand |
On 10/08/2020 15.06, Roman Bolshakov wrote: > For compatibility with other SVR4 assemblers, '/' starts a comment on > *-elf binutils target and thus division operator is not allowed [1][2]. > That breaks cstart64.S build: > > x86/cstart64.S: Assembler messages: > x86/cstart64.S:294: Error: unbalanced parenthesis in operand 1. > > The option is ignored on the Linux target of GNU binutils. > > 1. https://sourceware.org/binutils/docs/as/i386_002dChars.html > 2. https://sourceware.org/binutils/docs/as/i386_002dOptions.html#index-_002d_002ddivide-option_002c-i386 > > Cc: Cameron Esfahani <dirty@apple.com> > Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com> > --- > x86/Makefile | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/x86/Makefile b/x86/Makefile > index 8a007ab..22afbb9 100644 > --- a/x86/Makefile > +++ b/x86/Makefile > @@ -1 +1,3 @@ > include $(SRCDIR)/$(TEST_DIR)/Makefile.$(ARCH) > + > +COMMON_CFLAGS += -Wa,--divide Some weeks ago, I also played with an elf cross compiler and came to the same conclusion, that we need this option there. Unfortunately, it does not work with clang: https://gitlab.com/huth/kvm-unit-tests/-/jobs/707986800#L1629 You could try to wrap it with "cc-option" instead ... or use a proper check in the configure script to detect whether it's needed or not. And can you please put it next to the other COMMON_CFLAGS in x86/Makefile.common instead of x86/Makefile? Thanks, Thomas
On Fri, Aug 28, 2020 at 07:00:19AM +0200, Thomas Huth wrote: > On 10/08/2020 15.06, Roman Bolshakov wrote: > > For compatibility with other SVR4 assemblers, '/' starts a comment on > > *-elf binutils target and thus division operator is not allowed [1][2]. > > That breaks cstart64.S build: > > > > x86/cstart64.S: Assembler messages: > > x86/cstart64.S:294: Error: unbalanced parenthesis in operand 1. > > > > The option is ignored on the Linux target of GNU binutils. > > > > 1. https://sourceware.org/binutils/docs/as/i386_002dChars.html > > 2. https://sourceware.org/binutils/docs/as/i386_002dOptions.html#index-_002d_002ddivide-option_002c-i386 > > > > Cc: Cameron Esfahani <dirty@apple.com> > > Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com> > > --- > > x86/Makefile | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/x86/Makefile b/x86/Makefile > > index 8a007ab..22afbb9 100644 > > --- a/x86/Makefile > > +++ b/x86/Makefile > > @@ -1 +1,3 @@ > > include $(SRCDIR)/$(TEST_DIR)/Makefile.$(ARCH) > > + > > +COMMON_CFLAGS += -Wa,--divide > > Some weeks ago, I also played with an elf cross compiler and came to the > same conclusion, that we need this option there. Unfortunately, it does > not work with clang: > > https://gitlab.com/huth/kvm-unit-tests/-/jobs/707986800#L1629 > > You could try to wrap it with "cc-option" instead ... or use a proper > check in the configure script to detect whether it's needed or not. > Hi Thomas, Thanks for reviewing the series. I'll look into both options and will test with both gcc and clang afterwards. I can also update .travis.yml in a new patch to test the build on macOS. > And can you please put it next to the other COMMON_CFLAGS in > x86/Makefile.common instead of x86/Makefile? > Sure. Regards, Roman
On 28/08/2020 08.54, Roman Bolshakov wrote: > On Fri, Aug 28, 2020 at 07:00:19AM +0200, Thomas Huth wrote: >> On 10/08/2020 15.06, Roman Bolshakov wrote: >>> For compatibility with other SVR4 assemblers, '/' starts a comment on >>> *-elf binutils target and thus division operator is not allowed [1][2]. >>> That breaks cstart64.S build: >>> >>> x86/cstart64.S: Assembler messages: >>> x86/cstart64.S:294: Error: unbalanced parenthesis in operand 1. >>> >>> The option is ignored on the Linux target of GNU binutils. >>> >>> 1. https://sourceware.org/binutils/docs/as/i386_002dChars.html >>> 2. https://sourceware.org/binutils/docs/as/i386_002dOptions.html#index-_002d_002ddivide-option_002c-i386 >>> >>> Cc: Cameron Esfahani <dirty@apple.com> >>> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com> >>> --- >>> x86/Makefile | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/x86/Makefile b/x86/Makefile >>> index 8a007ab..22afbb9 100644 >>> --- a/x86/Makefile >>> +++ b/x86/Makefile >>> @@ -1 +1,3 @@ >>> include $(SRCDIR)/$(TEST_DIR)/Makefile.$(ARCH) >>> + >>> +COMMON_CFLAGS += -Wa,--divide >> >> Some weeks ago, I also played with an elf cross compiler and came to the >> same conclusion, that we need this option there. Unfortunately, it does >> not work with clang: >> >> https://gitlab.com/huth/kvm-unit-tests/-/jobs/707986800#L1629 >> >> You could try to wrap it with "cc-option" instead ... or use a proper >> check in the configure script to detect whether it's needed or not. >> > > Hi Thomas, > > Thanks for reviewing the series. I'll look into both options and will > test with both gcc and clang afterwards. I can also update .travis.yml > in a new patch to test the build on macOS. That would be great, thanks! Note that you need at least Clang v10 (the one from Fedora 32 is fine) to compile the kvm-unit-tests. And if it's of any help, this was the stuff that I used in .travis.yml for my experiments (might still be incomplete, though): - os: osx osx_image: xcode12 addons: homebrew: packages: - bash - coreutils - qemu - x86_64-elf-gcc env: - CONFIG="--cross-prefix=x86_64-elf-" - BUILD_DIR="build" - TESTS="umip" - ACCEL="tcg" - os: osx osx_image: xcode12 addons: homebrew: packages: - bash - coreutils - qemu - i386-elf-gcc env: - CONFIG="--arch=i386 --cross-prefix=x86_64-elf-" - BUILD_DIR="build" - TESTS="umip" - ACCEL="tcg" Thomas
On Fri, Aug 28, 2020 at 09:24:10AM +0200, Thomas Huth wrote: > On 28/08/2020 08.54, Roman Bolshakov wrote: > > On Fri, Aug 28, 2020 at 07:00:19AM +0200, Thomas Huth wrote: > >> On 10/08/2020 15.06, Roman Bolshakov wrote: > >>> For compatibility with other SVR4 assemblers, '/' starts a comment on > >>> *-elf binutils target and thus division operator is not allowed [1][2]. > >>> That breaks cstart64.S build: > >>> > >>> x86/cstart64.S: Assembler messages: > >>> x86/cstart64.S:294: Error: unbalanced parenthesis in operand 1. > >>> > >>> The option is ignored on the Linux target of GNU binutils. > >>> > >>> 1. https://sourceware.org/binutils/docs/as/i386_002dChars.html > >>> 2. https://sourceware.org/binutils/docs/as/i386_002dOptions.html#index-_002d_002ddivide-option_002c-i386 > >>> > >>> Cc: Cameron Esfahani <dirty@apple.com> > >>> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com> > >>> --- > >>> x86/Makefile | 2 ++ > >>> 1 file changed, 2 insertions(+) > >>> > >>> diff --git a/x86/Makefile b/x86/Makefile > >>> index 8a007ab..22afbb9 100644 > >>> --- a/x86/Makefile > >>> +++ b/x86/Makefile > >>> @@ -1 +1,3 @@ > >>> include $(SRCDIR)/$(TEST_DIR)/Makefile.$(ARCH) > >>> + > >>> +COMMON_CFLAGS += -Wa,--divide > >> > >> Some weeks ago, I also played with an elf cross compiler and came to the > >> same conclusion, that we need this option there. Unfortunately, it does > >> not work with clang: > >> > >> https://gitlab.com/huth/kvm-unit-tests/-/jobs/707986800#L1629 > >> > >> You could try to wrap it with "cc-option" instead ... or use a proper > >> check in the configure script to detect whether it's needed or not. > >> > > > > Hi Thomas, > > > > Thanks for reviewing the series. I'll look into both options and will > > test with both gcc and clang afterwards. I can also update .travis.yml > > in a new patch to test the build on macOS. > > That would be great, thanks! Note that you need at least Clang v10 (the > one from Fedora 32 is fine) to compile the kvm-unit-tests. > > And if it's of any help, this was the stuff that I used in .travis.yml > for my experiments (might still be incomplete, though): > > - os: osx > osx_image: xcode12 > addons: > homebrew: > packages: > - bash > - coreutils > - qemu > - x86_64-elf-gcc > env: > - CONFIG="--cross-prefix=x86_64-elf-" > - BUILD_DIR="build" > - TESTS="umip" > - ACCEL="tcg" > > - os: osx > osx_image: xcode12 > addons: > homebrew: > packages: > - bash > - coreutils > - qemu > - i386-elf-gcc It's going to be i686-elf-gcc. > env: > - CONFIG="--arch=i386 --cross-prefix=x86_64-elf-" `--cross-prefix=i686-elf-`, respectively. > - BUILD_DIR="build" > - TESTS="umip" > - ACCEL="tcg" > Thanks, I'll use it as the basis. Do I need to add your Signed-off-by: here? or Suggested-by: is enough? IIRC all tests pass on TCG/5.1. Regards, Roman
On 28/08/2020 09.47, Roman Bolshakov wrote: > On Fri, Aug 28, 2020 at 09:24:10AM +0200, Thomas Huth wrote: >> On 28/08/2020 08.54, Roman Bolshakov wrote: >>> On Fri, Aug 28, 2020 at 07:00:19AM +0200, Thomas Huth wrote: >>>> On 10/08/2020 15.06, Roman Bolshakov wrote: >>>>> For compatibility with other SVR4 assemblers, '/' starts a comment on >>>>> *-elf binutils target and thus division operator is not allowed [1][2]. >>>>> That breaks cstart64.S build: >>>>> >>>>> x86/cstart64.S: Assembler messages: >>>>> x86/cstart64.S:294: Error: unbalanced parenthesis in operand 1. >>>>> >>>>> The option is ignored on the Linux target of GNU binutils. >>>>> >>>>> 1. https://sourceware.org/binutils/docs/as/i386_002dChars.html >>>>> 2. https://sourceware.org/binutils/docs/as/i386_002dOptions.html#index-_002d_002ddivide-option_002c-i386 >>>>> >>>>> Cc: Cameron Esfahani <dirty@apple.com> >>>>> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com> >>>>> --- >>>>> x86/Makefile | 2 ++ >>>>> 1 file changed, 2 insertions(+) >>>>> >>>>> diff --git a/x86/Makefile b/x86/Makefile >>>>> index 8a007ab..22afbb9 100644 >>>>> --- a/x86/Makefile >>>>> +++ b/x86/Makefile >>>>> @@ -1 +1,3 @@ >>>>> include $(SRCDIR)/$(TEST_DIR)/Makefile.$(ARCH) >>>>> + >>>>> +COMMON_CFLAGS += -Wa,--divide >>>> >>>> Some weeks ago, I also played with an elf cross compiler and came to the >>>> same conclusion, that we need this option there. Unfortunately, it does >>>> not work with clang: >>>> >>>> https://gitlab.com/huth/kvm-unit-tests/-/jobs/707986800#L1629 >>>> >>>> You could try to wrap it with "cc-option" instead ... or use a proper >>>> check in the configure script to detect whether it's needed or not. >>>> >>> >>> Hi Thomas, >>> >>> Thanks for reviewing the series. I'll look into both options and will >>> test with both gcc and clang afterwards. I can also update .travis.yml >>> in a new patch to test the build on macOS. >> >> That would be great, thanks! Note that you need at least Clang v10 (the >> one from Fedora 32 is fine) to compile the kvm-unit-tests. >> >> And if it's of any help, this was the stuff that I used in .travis.yml >> for my experiments (might still be incomplete, though): >> >> - os: osx >> osx_image: xcode12 >> addons: >> homebrew: >> packages: >> - bash >> - coreutils >> - qemu >> - x86_64-elf-gcc >> env: >> - CONFIG="--cross-prefix=x86_64-elf-" >> - BUILD_DIR="build" >> - TESTS="umip" >> - ACCEL="tcg" >> >> - os: osx >> osx_image: xcode12 >> addons: >> homebrew: >> packages: >> - bash >> - coreutils >> - qemu >> - i386-elf-gcc > > It's going to be i686-elf-gcc. Ah, there are two flavours? Ok, good to know. >> env: >> - CONFIG="--arch=i386 --cross-prefix=x86_64-elf-" > > `--cross-prefix=i686-elf-`, respectively. > >> - BUILD_DIR="build" >> - TESTS="umip" >> - ACCEL="tcg" >> > > Thanks, I'll use it as the basis. Do I need to add your Signed-off-by: here? > or Suggested-by: is enough? Suggested-by is fine. > IIRC all tests pass on TCG/5.1. Yes, please tweak the TEST="..:" lines accordingly, I just added one test there for my initial tries. Thomas
On Fri, Aug 28, 2020 at 09:56:28AM +0200, Thomas Huth wrote: > On 28/08/2020 09.47, Roman Bolshakov wrote: > > On Fri, Aug 28, 2020 at 09:24:10AM +0200, Thomas Huth wrote: > >> On 28/08/2020 08.54, Roman Bolshakov wrote: > >>> On Fri, Aug 28, 2020 at 07:00:19AM +0200, Thomas Huth wrote: > >>>> On 10/08/2020 15.06, Roman Bolshakov wrote: > >>>>> For compatibility with other SVR4 assemblers, '/' starts a comment on > >>>>> *-elf binutils target and thus division operator is not allowed [1][2]. > >>>>> That breaks cstart64.S build: > >>>>> > >>>>> x86/cstart64.S: Assembler messages: > >>>>> x86/cstart64.S:294: Error: unbalanced parenthesis in operand 1. > >>>>> > >>>>> The option is ignored on the Linux target of GNU binutils. > >>>>> > >>>>> 1. https://sourceware.org/binutils/docs/as/i386_002dChars.html > >>>>> 2. https://sourceware.org/binutils/docs/as/i386_002dOptions.html#index-_002d_002ddivide-option_002c-i386 > >>>>> > >>>>> Cc: Cameron Esfahani <dirty@apple.com> > >>>>> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com> > >>>>> --- > >>>>> x86/Makefile | 2 ++ > >>>>> 1 file changed, 2 insertions(+) > >>>>> > >>>>> diff --git a/x86/Makefile b/x86/Makefile > >>>>> index 8a007ab..22afbb9 100644 > >>>>> --- a/x86/Makefile > >>>>> +++ b/x86/Makefile > >>>>> @@ -1 +1,3 @@ > >>>>> include $(SRCDIR)/$(TEST_DIR)/Makefile.$(ARCH) > >>>>> + > >>>>> +COMMON_CFLAGS += -Wa,--divide > >>>> > >>>> Some weeks ago, I also played with an elf cross compiler and came to the > >>>> same conclusion, that we need this option there. Unfortunately, it does > >>>> not work with clang: > >>>> > >>>> https://gitlab.com/huth/kvm-unit-tests/-/jobs/707986800#L1629 > >>>> > >>>> You could try to wrap it with "cc-option" instead ... or use a proper > >>>> check in the configure script to detect whether it's needed or not. > >>>> > >>> > >>> Hi Thomas, > >>> > >>> Thanks for reviewing the series. I'll look into both options and will > >>> test with both gcc and clang afterwards. I can also update .travis.yml > >>> in a new patch to test the build on macOS. > >> > >> That would be great, thanks! Note that you need at least Clang v10 (the > >> one from Fedora 32 is fine) to compile the kvm-unit-tests. > >> > >> And if it's of any help, this was the stuff that I used in .travis.yml > >> for my experiments (might still be incomplete, though): > >> > >> - os: osx > >> osx_image: xcode12 > >> addons: > >> homebrew: > >> packages: > >> - bash > >> - coreutils > >> - qemu > >> - x86_64-elf-gcc > >> env: > >> - CONFIG="--cross-prefix=x86_64-elf-" > >> - BUILD_DIR="build" > >> - TESTS="umip" > >> - ACCEL="tcg" > >> > >> - os: osx > >> osx_image: xcode12 > >> addons: > >> homebrew: > >> packages: > >> - bash > >> - coreutils > >> - qemu > >> - i386-elf-gcc > > > > It's going to be i686-elf-gcc. > > Ah, there are two flavours? Ok, good to know. > i386-elf-gcc package was configured as x86_64-pc-elf target and then was renamed to x86_64-elf-gcc to match GCC target name (last December or in January). x86_64_elf-gcc can't properly link 32-bit ELF binaries because of GCC PR16470 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=16470), PR32044 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=32044). A cross-compiler is needed for that, that's why i686-elf-gcc was added in June/May: https://github.com/Homebrew/homebrew-core/pull/54946/commits/94ee559e6c6284539f74662a85ec20c174436677 But as far as I understand i686-pc-elf target is the same as i386-pc-elf in GCC/binutils (according to code inspection and a following discussion freenode's #gcc channel). FWIW, i686-elf-gcc can be installed in parallel with x86_64-elf-gcc. Thanks, Roman
On Fri, Aug 28, 2020 at 07:00:19AM +0200, Thomas Huth wrote: > On 10/08/2020 15.06, Roman Bolshakov wrote: > > + > > +COMMON_CFLAGS += -Wa,--divide > > Some weeks ago, I also played with an elf cross compiler and came to the > same conclusion, that we need this option there. Unfortunately, it does > not work with clang: > > https://gitlab.com/huth/kvm-unit-tests/-/jobs/707986800#L1629 > > You could try to wrap it with "cc-option" instead ... or use a proper > check in the configure script to detect whether it's needed or not. > Hi Thomas, I've wrapped it but clang can't deal with another option: -Woverride-init Even if I wrap it with cc-option and add wrapped clang's -Winitializer-overrides, the build fails with: x86/vmexit.c:577:5: error: no previous prototype for function 'main' [-Werror,-Wmissing-prototypes] int main(int ac, char **av) ^ 1 error generated. <builtin>: recipe for target 'x86/vmexit.o' failed I'm puzzled with this one. CI log (ubuntu focal + clang 10): https://travis-ci.com/github/roolebo/kvm-unit-tests/jobs/379561410 Now I wonder if wrong clang is used... Perhaps I should try --cc=clang-10 in .travis.yml instead of --cc=clang. Thanks, Roman
On 31/08/2020 19.30, Roman Bolshakov wrote: > On Fri, Aug 28, 2020 at 07:00:19AM +0200, Thomas Huth wrote: >> On 10/08/2020 15.06, Roman Bolshakov wrote: >>> + >>> +COMMON_CFLAGS += -Wa,--divide >> >> Some weeks ago, I also played with an elf cross compiler and came to the >> same conclusion, that we need this option there. Unfortunately, it does >> not work with clang: >> >> https://gitlab.com/huth/kvm-unit-tests/-/jobs/707986800#L1629 >> >> You could try to wrap it with "cc-option" instead ... or use a proper >> check in the configure script to detect whether it's needed or not. >> > > Hi Thomas, > > I've wrapped it but clang can't deal with another option: > -Woverride-init > > Even if I wrap it with cc-option and add wrapped clang's > -Winitializer-overrides, the build fails with: > > x86/vmexit.c:577:5: error: no previous prototype for function 'main' > [-Werror,-Wmissing-prototypes] > int main(int ac, char **av) > ^ > 1 error generated. > <builtin>: recipe for target 'x86/vmexit.o' failed > > I'm puzzled with this one. > > CI log (ubuntu focal + clang 10): > https://travis-ci.com/github/roolebo/kvm-unit-tests/jobs/379561410 > > Now I wonder if wrong clang is used... Perhaps I should try > --cc=clang-10 in .travis.yml instead of --cc=clang. Hi Roman, yes, if you see the "no previous prototype for function 'main'" warning, your Clang is too old, you really need the latest and greatest version for the kvm-unit-tests. IIRC version 10 should be fine. Thomas
diff --git a/x86/Makefile b/x86/Makefile index 8a007ab..22afbb9 100644 --- a/x86/Makefile +++ b/x86/Makefile @@ -1 +1,3 @@ include $(SRCDIR)/$(TEST_DIR)/Makefile.$(ARCH) + +COMMON_CFLAGS += -Wa,--divide
For compatibility with other SVR4 assemblers, '/' starts a comment on *-elf binutils target and thus division operator is not allowed [1][2]. That breaks cstart64.S build: x86/cstart64.S: Assembler messages: x86/cstart64.S:294: Error: unbalanced parenthesis in operand 1. The option is ignored on the Linux target of GNU binutils. 1. https://sourceware.org/binutils/docs/as/i386_002dChars.html 2. https://sourceware.org/binutils/docs/as/i386_002dOptions.html#index-_002d_002ddivide-option_002c-i386 Cc: Cameron Esfahani <dirty@apple.com> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com> --- x86/Makefile | 2 ++ 1 file changed, 2 insertions(+)