Remove __mips32__ and __mips64__ defines from base/macros.h |
|||||
Issue descriptionr502712 added __mips32__ and __mips64__ defines in base/macros.h. Why is this needed there? Why can't ARCH_CPU_MIPS_FAMILY and friends from build/build_config.h be used instead?
,
Sep 14
,
Sep 14
Are there instructions somewhere for how to build for MIPS from Linux or macOS? If yes, I can try to help. Otherwise I can't because I will be working blindly and will likely just end up breaking the MIPS build.
,
Sep 20
This is the configuration that I usually use: mipsel: gn gen out/Release --args=" target_os=\"linux\" target_cpu=\"mipsel\" mips_arch_variant=\"r2\" is_debug=false" mips64el: gn gen out/Release --args=" target_os=\"linux\" target_cpu=\"mips64el\" mips_arch_variant=\"r2\" is_debug=false"
,
Sep 21
What is the reason for removing these defines? Maybe I can work on a different solution.
,
Sep 21
We already have a way to define CPU architecture in build/build_config.h. I don't believe it is necessary for MIPS to have its own separate way of doing something similar in base/macros.h.
,
Sep 21
re: comment 5 - Thanks! Is there a quick guide to building for MIPS somewhere, BTW? Looks like I also have to run: build/linux/sysroot_scripts/install-sysroot.py --arch=mipsel, but otherwise it is building...
,
Sep 21
+jorgelo from sandbox/linux/OWNERS for discussion. It looks like __mips32__ and __mips64__ are only used in sandbox/linux, where they want to fit in with __i386__ and __x86_64__. The issue is, __x86_64__ is defined by the compiler, but __mips64__ is not defined. So one possible solution is to switch sandbox/linux to use the defines from build/build_config.h, to be consistent with the rest of the code base. It will have to be a bit more verbose though. e.g. #if defined(__i386__) || defined(__x86_64__) || defined(__mips32__) becomes: #if defined(ARCH_CPU_X86_FAMILY) || (defined(ARCH_CPU_MIPS_FAMILY) && defined(ARCH_CPU_32_BITS)) Would that work? Another idea is to move __mips32__ and __mips64__ into a header in sandbox/linux.
,
Sep 24
Yes, #if (defined(ARCH_CPU_MIPS_FAMILY) && defined(ARCH_CPU_32_BITS)) will work. Let me know if you want us to upload the change.
,
Sep 24
Re: Comment 7: these are all the steps: mips32el: Get the sysroot: $ build/linux/sysroot_scripts/install-sysroot.py --arch=mipsel Configure and build: $ gn gen out/Release --args=" target_os=\"linux\" target_cpu=\"mipsel\" mips_arch_variant=\"r2\" is_debug=false" $ ninja -C out/Release chrome -j6 mips64el: Get the sysroot: $ build/linux/sysroot_scripts/install-sysroot.py --arch=mips64el Configure and build: $ gn gen out/Release --args=" target_os=\"linux\" target_cpu=\"mips64el\" mips_arch_variant=\"r2\" is_debug=false" $ ninja -C out/Release chrome -j6
,
Sep 24
I'm fine with switching the defines to the ones from build/build_config.h, as long as they mean the same thing.
,
Dec 4
What's the status here?
,
Dec 4
Someone needs to do a lot of search and replace. e.g. s/__x86_64__/ARCH_CPU_X86_64/ and s/__i386__/ARCH_CPU_X86/
,
Dec 4
Wait, I thought we said __x86_64__ and __i386__ are fine because they're compiler-defined and we only need to replace __mips32__ and __mips64__?
,
Dec 4
We did? Anyway, let's start with __mips32__ and __mips64__, so we can close out this bug. I'll take it.
,
Dec 15
https://chromium-review.googlesource.com/c/chromium/src/+/1362223 for those interested.
,
Dec 21
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1e78585880cc5f3dc73411b71fb9c7e81f0615c4 commit 1e78585880cc5f3dc73411b71fb9c7e81f0615c4 Author: Lei Zhang <thestig@chromium.org> Date: Fri Dec 21 22:58:50 2018 Remove __mips32__ and __mips64__ defines from base/macros.h. BUG= 882085 Change-Id: I07243c03a28a45f3d368269668b54eadc0b50e98 Reviewed-on: https://chromium-review.googlesource.com/c/1362223 Reviewed-by: Chris Palmer <palmer@chromium.org> Commit-Queue: Lei Zhang <thestig@chromium.org> Cr-Commit-Position: refs/heads/master@{#618650} [modify] https://crrev.com/1e78585880cc5f3dc73411b71fb9c7e81f0615c4/base/macros.h [modify] https://crrev.com/1e78585880cc5f3dc73411b71fb9c7e81f0615c4/sandbox/linux/bpf_dsl/linux_syscall_ranges.h [modify] https://crrev.com/1e78585880cc5f3dc73411b71fb9c7e81f0615c4/sandbox/linux/bpf_dsl/seccomp_macros.h [modify] https://crrev.com/1e78585880cc5f3dc73411b71fb9c7e81f0615c4/sandbox/linux/bpf_dsl/syscall_set.cc [modify] https://crrev.com/1e78585880cc5f3dc73411b71fb9c7e81f0615c4/sandbox/linux/seccomp-bpf-helpers/baseline_policy.cc [modify] https://crrev.com/1e78585880cc5f3dc73411b71fb9c7e81f0615c4/sandbox/linux/seccomp-bpf-helpers/baseline_policy_android.cc [modify] https://crrev.com/1e78585880cc5f3dc73411b71fb9c7e81f0615c4/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc [modify] https://crrev.com/1e78585880cc5f3dc73411b71fb9c7e81f0615c4/sandbox/linux/seccomp-bpf-helpers/syscall_sets.cc [modify] https://crrev.com/1e78585880cc5f3dc73411b71fb9c7e81f0615c4/sandbox/linux/seccomp-bpf-helpers/syscall_sets.h [modify] https://crrev.com/1e78585880cc5f3dc73411b71fb9c7e81f0615c4/sandbox/linux/seccomp-bpf/syscall.cc [modify] https://crrev.com/1e78585880cc5f3dc73411b71fb9c7e81f0615c4/sandbox/linux/syscall_broker/broker_process.cc [modify] https://crrev.com/1e78585880cc5f3dc73411b71fb9c7e81f0615c4/sandbox/linux/system_headers/linux_signal.h [modify] https://crrev.com/1e78585880cc5f3dc73411b71fb9c7e81f0615c4/sandbox/linux/system_headers/linux_syscalls.h
,
Dec 21
,
Dec 22
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d6db857be00802afb220199f1792951b4d155b14 commit d6db857be00802afb220199f1792951b4d155b14 Author: Lei Zhang <thestig@chromium.org> Date: Sat Dec 22 06:07:14 2018 Fix bad __mips64__ conversions from r618650. !defined(__mips64__) got mechanically converted to: !defined(ARCH_CPU_MIPS_FAMILY) && defined(ARCH_CPU_64_BITS) when it should have been: !(defined(ARCH_CPU_MIPS_FAMILY) && defined(ARCH_CPU_64_BITS)) BUG= 882085 TBR=palmer@chromium.org Change-Id: I1a21803da3d72965fa3ab113a9e54005c1d33e93 Reviewed-on: https://chromium-review.googlesource.com/c/1389796 Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: Lei Zhang <thestig@chromium.org> Cr-Commit-Position: refs/heads/master@{#618734} [modify] https://crrev.com/d6db857be00802afb220199f1792951b4d155b14/sandbox/linux/seccomp-bpf-helpers/syscall_sets.cc |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by palmer@chromium.org
, Sep 8Labels: OS-Android