Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sim: detect clang native compiler on MacOS. #5452

Merged
merged 1 commit into from Feb 11, 2022
Merged

sim: detect clang native compiler on MacOS. #5452

merged 1 commit into from Feb 11, 2022

Conversation

ptka
Copy link
Contributor

@ptka ptka commented Feb 10, 2022

Summary

Detect clang native compiler on MacOS.
In case clang based compiler is used, skip objcopy step in final build step.

Impact

Simulation on MacOS.

Testing

MacBook Pro Late 2013 running Big Sur (X86_64)
Testing on MacBook Air 2020 running Monterey (ARM64) will be part of separate PR.

@yamt
Copy link
Contributor

yamt commented Feb 10, 2022

X86_64 based hosts stay on current (gcc/binutils) configuration for backward compatibility.

i guess you misunderstand the situation.

i have always been using clang to develop sim/macOS/x86-64.
i have not used gcc for the purpose.
i don't know if it's possible to build sim/macOS with gcc.

also, i usually use clang to build sim/Linux/x86-64 without problems.
so i'm not sure why this option is necessary.

@ptka
Copy link
Contributor Author

ptka commented Feb 10, 2022

Native compiler on MacOS doesn't have objcopy installed. I assume, you have additional software installed on your MacOS which (maybe as a side-effect) installs objcopy. Can you check your system regarding this point, please.
If current MacOS/X86 is designed to use clang, then the option is most likely not needed, as it was just introduced for backward compatibility.

@yamt
Copy link
Contributor

yamt commented Feb 10, 2022

Native compiler on MacOS doesn't have objcopy installed. I assume, you have additional software installed on your MacOS which (maybe as a side-effect) installs objcopy. Can you check your system regarding this point, please. If current MacOS/X86 is designed to use clang, then the option is most likely not needed, as it was just introduced for backward compatibility.

yes, it requires objcopy from binutils.
see #1817
maybe we forgot to document it.
at that point, the llvm version of objcopy didn't work well.

can you explain why objcopy is not necessary for clang?

@xiaoxiang781216
Copy link
Contributor

xiaoxiang781216 commented Feb 10, 2022

@ptka does llvm-objcpy and llvm-objdump exist on macOS?

@yamt
Copy link
Contributor

yamt commented Feb 10, 2022

@ptka does llvm-objcpy and llvm-objdump exist on macOS?

in my environment with Monterey x86-64, Xcode 13.2.1,
neither Xcode nor CommandLineTools has them.

llvm from homebrew has them.

@yamt
Copy link
Contributor

yamt commented Feb 10, 2022

maybe we can consider to revert #1817 .
it's awkward to require binutils.

@xiaoxiang781216
Copy link
Contributor

@ptka found that objcopy isn't really needed, I think.

@ptka
Copy link
Contributor Author

ptka commented Feb 10, 2022

Native compiler on MacOS doesn't have objcopy installed. I assume, you have additional software installed on your MacOS which (maybe as a side-effect) installs objcopy. Can you check your system regarding this point, please. If current MacOS/X86 is designed to use clang, then the option is most likely not needed, as it was just introduced for backward compatibility.

yes, it requires objcopy from binutils. see #1817 maybe we forgot to document it. at that point, the llvm version of objcopy didn't work well.

can you explain why objcopy is not necessary for clang?

Afaik: objcopy is used to hide the NuttX world from the HOST world to avoid, that the wrong functions are called. Before linking the final host executable, objcopy renames a list of NuttX symbols. Overall this is a kind of a workaround as in principle is should work in a different way: Compiling code with "-fvisibility=hidden" should mark functions as hidden. Hidden means, they are not exported from any 'ld' created object file. All symbols, which needs to exported, can overwrite the visibility in the code itself. This way 'main()' and 'up_doirq()' gets visible in the HOST world although they are defined in the NuTTX world. On MacOS/clang the compiler/linker behaves exactly that way and objcopy is thus not needed.

@yamt
Copy link
Contributor

yamt commented Feb 10, 2022

Native compiler on MacOS doesn't have objcopy installed. I assume, you have additional software installed on your MacOS which (maybe as a side-effect) installs objcopy. Can you check your system regarding this point, please. If current MacOS/X86 is designed to use clang, then the option is most likely not needed, as it was just introduced for backward compatibility.

yes, it requires objcopy from binutils. see #1817 maybe we forgot to document it. at that point, the llvm version of objcopy didn't work well.
can you explain why objcopy is not necessary for clang?

Afaik: objcopy is used to hide the NuttX world from the HOST world to avoid, that the wrong functions are called. Before linking the final host executable, objcopy renames a list of NuttX symbols. Overall this is a kind of a workaround as in principle is should work in a different way: Compiling code with "-fvisibility=hidden" should mark functions as hidden. Hidden means, they are not exported from any 'ld' created object file. All symbols, which needs to exported, can overwrite the visibility in the code itself. This way 'main()' and 'up_doirq()' gets visible in the HOST world although they are defined in the NuTTX world. On MacOS/clang the compiler/linker behaves exactly that way and objcopy is thus not needed.

ok. thank you for the explanation.

  • i guess more functions need to be exported. eg. netdriver_setmacaddr
  • i don't think it's arm specific.
  • if it's clang-specific (i dunno) it might be simpler to use ifneq "$(shell $(CC) --version | grep clang)" "" than a kconfig
  • i don't understand the change in up_vfork_x86_64.S. is it somehow tied to the objcopy part of the PR? otherwise, i'd suggest to leave it for a separate PR.

@ptka
Copy link
Contributor Author

ptka commented Feb 10, 2022

maybe we can consider to revert #1817 .
it's awkward to require binutils.

I think, we need to distinguish between clang and gcc/binutils based HOST compiler suites (maybe it's even just an MacOS issue). On gcc/binutils based HOST compiler suites objcopy is needed to avoid wrong function calls between the NuttX and the HOST world within the overall simulation. In my opinion we cannot simply revert mentioned PR.

@yamt
Copy link
Contributor

yamt commented Feb 10, 2022

maybe we can consider to revert #1817 .
it's awkward to require binutils.

I think, we need to distinguish between clang and gcc/binutils based HOST compiler suites (maybe it's even just an MacOS issue). On gcc/binutils based HOST compiler suites objcopy is needed to avoid wrong function calls between the NuttX and the HOST world within the overall simulation. In my opinion we cannot simply revert mentioned PR.

as far as your approach works, there is no point to revert.

@xiaoxiang781216
Copy link
Contributor

If @ptka finding is right, I would prefer that we remove objcopy step for macOS regardless whether it is run on x64 or arm64.

@ptka
Copy link
Contributor Author

ptka commented Feb 10, 2022

Native compiler on MacOS doesn't have objcopy installed. I assume, you have additional software installed on your MacOS which (maybe as a side-effect) installs objcopy. Can you check your system regarding this point, please. If current MacOS/X86 is designed to use clang, then the option is most likely not needed, as it was just introduced for backward compatibility.

yes, it requires objcopy from binutils. see #1817 maybe we forgot to document it. at that point, the llvm version of objcopy didn't work well.
can you explain why objcopy is not necessary for clang?

Afaik: objcopy is used to hide the NuttX world from the HOST world to avoid, that the wrong functions are called. Before linking the final host executable, objcopy renames a list of NuttX symbols. Overall this is a kind of a workaround as in principle is should work in a different way: Compiling code with "-fvisibility=hidden" should mark functions as hidden. Hidden means, they are not exported from any 'ld' created object file. All symbols, which needs to exported, can overwrite the visibility in the code itself. This way 'main()' and 'up_doirq()' gets visible in the HOST world although they are defined in the NuTTX world. On MacOS/clang the compiler/linker behaves exactly that way and objcopy is thus not needed.

ok. thank you for the explanation.

  • i guess more functions need to be exported. eg. netdriver_setmacaddr
  • i don't think it's arm specific.
  • if it's clang-specific (i dunno) it might be simpler to use ifneq "$(shell $(CC) --version | grep clang)" "" than a kconfig
  • i don't understand the change in up_vfork_x86_64.S. is it somehow tied to the objcopy part of the PR? otherwise, i'd suggest to leave it for a separate PR.

This ifneq "$(shell $(CC) --version | grep clang)" "" could be a good approach. I reworked the PR.

@ptka
Copy link
Contributor Author

ptka commented Feb 10, 2022

@ptka does llvm-objcpy and llvm-objdump exist on macOS?

in my environment with Monterey x86-64, Xcode 13.2.1, neither Xcode nor CommandLineTools has them.

llvm from homebrew has them.

I would like to see MacOS based simulation independent from homebrew. There's nothing wrong with homebrew, but native compiler suite should be good enough, so there's no need for this dependencies from compiler perspective.

@ptka ptka changed the title sim: enable selecting clang native compiler on MacOS. sim: detect clang native compiler on MacOS. Feb 10, 2022
@xiaoxiang781216
Copy link
Contributor

more symbol need add default visibility: _up_cpu_started, _up_init_ipi, _netdriver_setmacaddr.

@ptka
Copy link
Contributor Author

ptka commented Feb 10, 2022

more symbol need add default visibility: _up_cpu_started, _up_init_ipi, _netdriver_setmacaddr.

that's the easy part ... the critical part is getting the modules compiled ... there's a hard dependency:

MODULECC = x86_64-elf-gcc
MODULELD = x86_64-elf-ld

to get the elf modules build.

This could be really a show-stopper (or at least an not-easy to fix dependency to homebrew based additional software needed on MacOS).

arch/sim/Kconfig Outdated Show resolved Hide resolved
arch/sim/Kconfig Outdated Show resolved Hide resolved
@ptka ptka closed this Feb 10, 2022
@ptka
Copy link
Contributor Author

ptka commented Feb 10, 2022

Let me park this PR completely ... it's not working, the way it was intended ... I need to re-think.

@xiaoxiang781216
Copy link
Contributor

xiaoxiang781216 commented Feb 10, 2022

more symbol need add default visibility: _up_cpu_started, _up_init_ipi, _netdriver_setmacaddr.

that's the easy part ... the critical part is getting the modules compiled ... there's a hard dependency:

MODULECC = x86_64-elf-gcc
MODULELD = x86_64-elf-ld

to get the elf modules build.

This could be really a show-stopper (or at least an not-easy to fix dependency to homebrew based additional software needed on MacOS).

Yes, elf need gcc toolchain, but this is an advance feature, not used by most people. It's reasonable require the user to install gcc toolchain if they want to try elf.

@ptka ptka reopened this Feb 10, 2022
@ptka
Copy link
Contributor Author

ptka commented Feb 10, 2022

I installed the packages via homebrew to fulfill:

MODULECC = x86_64-elf-gcc
MODULELD = x86_64-elf-ld

Additionally, I removed all changed except the "objcopy" part and the related symbol export changes. Making this PR focus on a single topic.

@xiaoxiang781216
Copy link
Contributor

@yamt do you ok with the change?

@xiaoxiang781216 xiaoxiang781216 merged commit 271518a into apache:master Feb 11, 2022
@@ -285,11 +285,14 @@ nuttx-names.dat: nuttx-names.in
nuttx$(EXEEXT): libarch$(LIBEXT) board/libboard$(LIBEXT) $(HEADOBJ) $(LINKOBJS) $(HOSTOBJS) nuttx-names.dat
$(Q) echo "LD: nuttx$(EXEEXT)"
$(Q) $(LD) -r $(LDLINKFLAGS) $(RELPATHS) $(EXTRA_LIBPATHS) -o nuttx.rel $(REQUIREDOBJS) $(LDSTARTGROUP) $(RELLIBS) $(EXTRA_LIBS) $(LDENDGROUP)
ifeq ("$(shell $(CC) --version | grep clang)","")
# none clang based native compilers need opjcopy to build simulation
$(Q) $(OBJCOPY) --redefine-syms=nuttx-names.dat nuttx.rel
$(Q) $(CC) $(CCLINKFLAGS) -Wl,-verbose 2>&1 | \
sed -e '/====/,/====/!d;//d' -e 's/__executable_start/_stext/g' -e 's/__init_array_start/_sinit/g' \
-e 's/__init_array_end/_einit/g' -e 's/__fini_array_start/_sfini/g' -e 's/__fini_array_end/_efini/g' >nuttx.ld
$(Q) echo "__init_array_start = .; __init_array_end = .; __fini_array_start = .; __fini_array_end = .;" >>nuttx.ld
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the init array stuff is about linux, not clang/gcc.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, gcc on macOS go the dynamical replacement?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my concern is clang on linux. i can't think of a reason why the init array is not necessary in that case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jerpelea jerpelea added this to To-Add in Release Notes - 10.3.0 Mar 15, 2022
@jerpelea jerpelea moved this from To-Add to Minor in Release Notes - 10.3.0 Mar 17, 2022
@jerpelea jerpelea moved this from Minor to In Progress in Release Notes - 10.3.0 Mar 17, 2022
@jerpelea jerpelea moved this from In Progress to added in Release Notes - 10.3.0 Mar 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants