I know the answer to this question is always no, but... could this be a bug in the compiler?

We have

template <typename T>
class : MyClass : public OtherClass {
  ...

  static const int kConstant = 5;
};


We only use MyClass<std::string> and no other type, in the same .cc file (still made sense to do a template)... so shouldn't that inline initialization be defined? (we are using C++11).

I uploaded this CL to fix the issue anyway: https://android-review.googlesource.com/229660

I think it's designed by C++:

Adapted from C++/14 9.4.2.3:
(const static member) ... The member shall still be defined in a namespace scope if it is odr-used (3.2) in the program and the namespace scope definition shall not contain an initializer.

9.4.2.4:
[ Note: There shall be exactly one definition of a static data member that is odr-used (3.2) in a program; no diagnostic is required. — end note ] Unnamed classes and classes contained directly or indirectly within unnamed classes shall not contain static data members.

So a const static member should be defined out-of-line when it's odr-used (e.g. taken address).

Sometimes, the compiler doesn't complain when the code doesn't follow the rules. I guess that's because all of the uses are optimized out. For example, I got a linkage error of this when compiling with -O0 but not with -O2:

////////////////////////////
void fun(const int &bar) {
}

class Foo {
public:
  const static int bar = 0;
};

int main() {
  fun(Foo().bar);
  return 0;
}
////////////////////////////

No, this is not a bug in the compiler :)

probably ASAN is taking the address of that variable and that is why now the compiler complains. 
the fix you provided is fine but you could keep the variable as a static member if you provide a real definition outside the class.

You see? I told you it wasn't a compiler bug.

https://android-review.googlesource.com/#/c/229660/ landed.

The builders are still failing. Should the chrome os side be updated as well? Or would the changes propagate automatically in a few days?

They didn't land in CrOS yet. I was hoping to get the CQ issues resolved today. I'll update here.

This bug started to appear in other non-asan builds:

https://uberchromegw.corp.google.com/i/chromiumos.tryserver/builders/full/builds/118/steps/BuildPackages/logs/stdio

Hi, sheriff here!  The ASAN builders on the external waterfall have been red since May 11.  Does the cros ebuild need to be updated to a more recent version of update_engine, or something like that?

Also, I thought we were moving all those repos back to the CrOS side, but maybe not this one?

Thanks!

deymo? any updates?

We need to fix this asap, these builders have been failing since May 12. I can try to help if you can't get to this today. Thanks!

 Issue 614084  has been merged into this issue.

+adlr

Didn't this get fixed?

Apparently not, see: https://build.chromium.org/p/chromiumos.chromium/builders/amd64-generic-tot-asan-informational/builds/9310

Output from BuildPackages:

update_engine-0.0.3-r1906: [115/115] LINK update_engine
update_engine-0.0.3-r1906: FAILED: flock linker.lock x86_64-cros-linux-gnu-clang++ -Wl,--gc-sections -Wl,-O1 -Wl,-O2 -Wl,--as-needed -fsanitize=address -fsanitize=alignment -fsanitize=shift -Wl,-z,relro -Wl,-z,noexecstack -Wl,-z,now -Wl,--as-needed --sysroot=/build/amd64-generic -pthread -pie -o update_engine -Wl,--start-group aosp/system/update_engine/update_engine.main.o aosp/system/update_engine/libupdate_metadata-protos.a aosp/system/update_engine/libpayload_consumer.a aosp/system/update_engine/libupdate_engine.a  -Wl,--end-group -lbrillo-381699 -lbase-381699 -ldbus-1 -lmetrics-381699 -lexpat -lcrypto -lcurl -limgpatch -lbz2 -lz -lssl -lxz-embedded -lprotobuf-lite -lpthread -lbz2 -lpolicy-381699 -lrootdev -lrt -lvboot_host
update_engine-0.0.3-r1906: /build/amd64-generic/usr/include/base-381699/base/logging.h:592: error: undefined reference to 'chromeos_update_manager::(anonymous namespace)::RetryPollVariable<std::string>::kRetryIntervalSeconds'
update_engine-0.0.3-r1906: /build/amd64-generic/usr/include/base-381699/base/logging.h:592: error: undefined reference to 'chromeos_update_manager::(anonymous namespace)::RetryPollVariable<std::string>::kRetryIntervalSeconds'
update_engine-0.0.3-r1906: /build/amd64-generic/usr/include/base-381699/base/logging.h:592: error: undefined reference to 'chromeos_update_manager::(anonymous namespace)::RetryPollVariable<std::string>::kRetryIntervalSeconds'
update_engine-0.0.3-r1906: /build/amd64-generic/usr/include/base-381699/base/logging.h:592: error: undefined reference to 'chromeos_update_manager::(anonymous namespace)::RetryPollVariable<std::string>::kRetryIntervalSeconds'

Update engine was forked at
aosp/platform/system/update_engine
 * 46a9aae Fix non-critical updates on boards without an OOBE flow. (Alex Deymo, Mon, 9 May 2016 11:29:05 -0700)

I guess it has not gotten the fix in CrOS, as the fix landed May 13th
https://android-review.googlesource.com/#/q/project:platform/system/update_engine

I can see the missing commits in cros/upstream. Deymo, what do you suggest the uprev flow for CrOS is now that you have unleashed us?

repo cherry-pick whatever fix you need. I just uploaded this cherry-pick here:

https://chromium-review.googlesource.com/346773

If you want to take all the CLs and deal with all the eventual minor fixes due to differences in the toolchains and build systems, you can also do a git merge (CQ doesn't handle git merges unless you stash them.. I'll send a feature request for that).

For taking changes note that:
a) I don't have +2 on these repos anymore in CrOS.
b) Brillo team is not gonna be doing testing on chromebooks for the UE in AOSP; but we won't also do any intentional breaking changes (like removing CrOS-only code just because is not compiled in AOSP). All the #ifdef ANDROID/CHROMEOS will remain and patches to fix required new ones are welcome.

I meant "squash". Here's the FR for git merges:  crbug.com/614164 

Deymo, if I +2 that cherry pick above ( https://chromium-review.googlesource.com/#/c/346773/1 ), will that fix this urgent issue?

Thanks!

Yes, we should land this for now (I did +2) and figure out how to deal with the fork long term.

Still happening.

The CL is still in the CQ. If it doesn't get through this time I will chump this.

Thanks ihf@ for chumping the CL: https://chromium-review.googlesource.com/#/c/346773/

Should be fixed in next run.

Thanks a bunch!