tree 098e52632757556514ab8bc7adf6b3a58b7a698e
parent ba3b7ecf04099620c6087451156a1793dcc3607f
author Peter Kasting <pkasting@chromium.org> 1596516800 +0000
committer Commit Bot <commit-bot@chromium.org> 1596516800 +0000

Fix raw_logging.h to allow deterministic symbols, and regenerate .defs.

__FILE__ is replaced by a string constant, whose type is const char[n],
where 'n' is the length of the actual filename.  Because
ABSL_INTERNAL_LOG() passes __FILE__ to the templated function
AtomicHook::operator()(Args&...), the value of 'n' is encoded into the
mangled name of the generated operator.  As a result, generated .def
files containing an entry for this operator would result in "symbol not
found" errors during link on machines which used a different-length
filename for such a call.

This was not previously a problem because for some reason this
particular symbol was omitted from the generated .defs.  As part of
trying to fix the win32-archive-dbg builder, which failed due to
not-found symbols after crrev.com/793680, I saw that no updates to these
files had been applied during the last couple of Abseil rolls, suspected
they might be out of date, and regenerated the files.  This caused more
builds to break rather than fewer since this symbol was now being
included.

It's possible this has something to do with absolute paths (see
blog.llvm.org/2019/11/deterministic-builds-with-clang-and-lld.html ),
but I don't think so; we should be using relative paths to compile.
Plus, the failed builds were on configurations that did not enable
is_official_build, and when that's not enabled, we should be setting
strip_absolute_paths_from_debug_symbols as well.  (That might be
irrelevant; I'm overly ignorant of how object files encode info.)  I am
guessing the core problem is that if multiple files invoke
ABSL_INTERNAL_LOG(), then even with relative file paths, the generated
symbol effectively depends on which file ends up instantiating this
first, or gets selected by the linker, or whatever.

So this patch modifies the macro to explicitly cast __FILE__ to a
pointer (the type that will be passed to the ultimate destination
anyway), making the symbol name for the operator deterministic.  If we
think this patch is a good idea, we should try to upstream it.

The .def files have been subsequently regenerated (and got one other
symbol added, so maybe they did omit something important?), and I added
a note to README.chromium about running this script, along with trying
to update that file more generally.

Bug: 1103706
Change-Id: I39ba72b4dd688340cacf0a814d6f461a96f891e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2333649
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#794391}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 2a15c32108ed6c80bb75b529e027860c17219553