New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 716549 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

LLVM: force use gcc to build ec-utils

Project Member Reported by shurst@google.com, Apr 28 2017

Issue description

We need to continue to use gcc to build ec-utils while we are migrating to LLVM

Building ec-utils with clang results in the following error:

__visible void *memset(void *dest, int c, size_t len);
^
include/compile_time_macros.h:30:34: note: expanded from macro '__visible'
#define __visible       __attribute__((externally_visible))

Fix:

Add ../env/chromeos-base/ec-utils file containing the following:

cros_pre_src_prepare_use_gcc() {
        cros_use_gcc
}

 
Cc: g...@chromium.org
it seems to me the error pasted is not complete? 

can you please paste the full error?
if possible, we should not go back to GCC.


Comment 2 by shurst@google.com, Apr 28 2017

ec-utils-0.0.1-r3770:  * Building for boards: chell chell_pd
ec-utils-0.0.1-r3770: make -j16 utils-host 
ec-utils-0.0.1-r3770:   HOSTCC  util/ectool
ec-utils-0.0.1-r3770:   HOSTCC  util/lbplay
ec-utils-0.0.1-r3770:   HOSTCC  util/stm32mon
ec-utils-0.0.1-r3770:   HOSTCC  util/ec_sb_firmware_update
ec-utils-0.0.1-r3770:   HOSTCC  util/lbcc
ec-utils-0.0.1-r3770:   HOSTCC  util/ec_parse_panicinfo
ec-utils-0.0.1-r3770: x86_64-cros-linux-gnu-clang  -DOUTDIR=build/chell/ -DCHIP=mec1322 -DBOARD_TASKFILE=ec.tasklist -DBOARD=chell -DCORE=cortex-m -DPROJECT=ec -DCHIP_VARIANT= -DCHIP_FAMILY= -DBOARD_CHELL -DCHIP_MEC1322 -DCORE_CORTEX_M -DCHIP_VARIANT_ -DCHIP_FAMILY_ -DFINAL_OUTDIR=build/chell -Iinclude  -Icore/cortex-m/include  -Icore/cortex-m  -Ichip/mec1322  -Iboard/chell  -Icommon  -Ipower  -Itest  -Icts/common  -Icts/  -Iprivate  -Idriver  -Idriver/temp_sensor  -Idriver/led  -Idriver/tcpm  -Idriver/battery  -Idriver/charger  -Ibuild/chell  -Itest  -I.      -DTEST_ec -DTEST_EC    -DSECTION_IS_ -DSECTION=  -O3 -g -Wall -Werror -Wundef -Wno-trigraphs -fno-strict-aliasing -fno-common -Werror-implicit-function-declaration -Wno-format-security -fno-strict-overflow -Wstrict-prototypes -Wdeclaration-after-statement -Wno-pointer-sign -DHOST_TOOLS_BUILD -ffreestanding -fno-builtin -nostdinc -Ibuiltin/ -D"__keep= " \
ec-utils-0.0.1-r3770: 			-MMD -MF build/chell/util/usb_pd_policy.o.d -c board/chell/usb_pd_policy.c -flto -o build/chell/util/usb_pd_policy.o
ec-utils-0.0.1-r3770: In file included from board/chell/usb_pd_policy.c:17:
ec-utils-0.0.1-r3770: include/util.h:71:1: error: unknown attribute 'externally_visible' ignored [-Werror,-Wunknown-attributes]
ec-utils-0.0.1-r3770: __visible void *memset(void *dest, int c, size_t len);
ec-utils-0.0.1-r3770: ^
ec-utils-0.0.1-r3770: include/compile_time_macros.h:30:34: note: expanded from macro '__visible'
ec-utils-0.0.1-r3770: #define __visible       __attribute__((externally_visible))
ec-utils-0.0.1-r3770:                                        ^
ec-utils-0.0.1-r3770: 1 error generated.
ec-utils-0.0.1-r3770: make: *** [util/build.mk:36: build/chell/util/usb_pd_policy.o] Error 1
ec-utils-0.0.1-r3770: make: *** Waiting for unfinished jobs....
ec-utils-0.0.1-r3770:  * ERROR: chromeos-base/ec-utils-0.0.1-r3770::chromiumos failed (compile phase):
ec-utils-0.0.1-r3770:  *   emake failed

Comment 3 by vpalatin@google.com, Apr 28 2017

>  can you please paste the full error?
> if possible, we should not go back to GCC.

the same code is built once with GCC by the chromeos-ec package, then by clang by the ec-utils package.
if possible, we should really build it with the same compiler to avoid issues creeping after submission as this one (and we decide to go for now with GCC for the EC firmware)

Comment 4 by vapier@chromium.org, Apr 28 2017

i suspect there's some target code that doesn't make sense for host tools.  unless the ec codebase is always defining its own copy of memset (not just a prototype, but also its own implementation) ?

Comment 5 by vpalatin@google.com, Apr 28 2017

> i suspect there's some target code that doesn't make sense for host tools.  
> unless the ec codebase is always defining its own copy of memset
>  (not just a prototype, but also its own implementation) ?

It's a bit more complicated here: a bit of target code is built for the build platform to generate some data.
but I would stick to my recommendation in #3, can we keep it simple and build chromeos-ec and ec-utils rather than debating ?

Comment 6 by g...@chromium.org, Apr 28 2017

(if we want to keep this gcc only, ignore me :) )

looking through git history, the commit that added __visible (bc404c94b4) mentions "Due to very eager optimization, we have to give gcc a little notch to not kick out memset," so it seems that this attribute is only tagged on memset to work around an issue with GCC.

memset seems to be the only user of __visible in ec. there's also __keep that uses both the externally_visible and used attributes, but:
- i haven't checked to see if a function tagged with __keep is actually included when building utils-host, and
- skimming through docs, it looks like `#define __keep __attribute__((used))` would get you the same effect in clang anyway
Project Member

Comment 7 by bugdroid1@chromium.org, May 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/446ec2dfa9b9fdec1b016dc4e149b9f388bb2fc6

commit 446ec2dfa9b9fdec1b016dc4e149b9f388bb2fc6
Author: Sam Hurst <shurst@google.com>
Date: Mon May 01 19:20:06 2017

profile: use gcc to build ec-utils

We need to continue to use gcc to build ec-utils while we
are migrating to LLVM

BUG= chromium:716549 
TEST=this package will be built with gcc.

Change-Id: Ie62975bf20d05ef85fde6609d6058deec6081036
Reviewed-on: https://chromium-review.googlesource.com/490706
Commit-Ready: Sam Hurst <shurst@google.com>
Tested-by: Sam Hurst <shurst@chromium.org>
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>

[add] https://crrev.com/446ec2dfa9b9fdec1b016dc4e149b9f388bb2fc6/chromeos/config/env/chromeos-base/ec-utils

Project Member

Comment 8 by sheriffbot@chromium.org, Jul 18 2017

Labels: Hotlist-Google
Status: Verified (was: Assigned)
ec-utils is using gcc for a while. Closing this.

Sign in to add a comment