DevHeads.net

Chromium C++ help needed

Hi folks,

I spent some time this weekend trying to get Chromium 72 building on
Fedora, but I kept running into a C++ issue that I was not able to resolve.
This happened with gcc-9.0.1-0.8.fc30.x86_64 and gcc-8.3.1-2.fc29.x86_64.

Here's a sample of the error (it happens in a few places), from Fedora 30:

In file included from
../../base/trace_event/trace_event_system_stats_monitor.h:15,
from ../../base/trace_event/trace_event.h:26,
from ../../base/threading/scoped_blocking_call.cc:11:
../../base/trace_event/trace_log.h: In constructor
'base::ScopedBlockingCall::ScopedBlockingCall(base::BlockingType)':
../../base/threading/scoped_blocking_call.cc:88:5: in 'constexpr'
expansion of
'base::trace_event::TraceLog::GetBuiltinCategoryEnabled(((const
char*)"base"))'
../../base/trace_event/trace_log.h:215:25: error: '((&
base::trace_event::CategoryRegistry::categories_[7]) != 0)' is not a
constant expression
215 | if (builtin_category)
| ^

Now, chromium isn't the easiest code base to work with, but what seems to
be happening is that this code calls one of the TRACE_EVENT macros, like
this from base/threading/scoped_blocking_call.cc:

TRACE_EVENT_BEGIN1("base", "ScopedBlockingCall", "blocking_type",
static_cast<int>(blocking_type));

Those macros are defined in base/trace_event/common/trace_event_common.h:

#define TRACE_EVENT_BEGIN1(category_group, name, arg1_name, arg1_val) \
INTERNAL_TRACE_EVENT_ADD(TRACE_EVENT_PHASE_BEGIN, category_group, name, \
TRACE_EVENT_FLAG_NONE, arg1_name, arg1_val)

INTERNAL_TRACE_EVENT_ADD() is defined in base/trace_event/trace_event.h:

/ Implementation detail: internal macro to create static category and add
// event if the category is enabled.
#define INTERNAL_TRACE_EVENT_ADD(phase, category_group, name, flags, ...) \
do { \
INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO(category_group); \
if (INTERNAL_TRACE_EVENT_CATEGORY_GROUP_ENABLED()) { \
trace_event_internal::AddTraceEvent( \
phase, INTERNAL_TRACE_EVENT_UID(category_group_enabled), name, \
trace_event_internal::kGlobalScope, trace_event_internal::kNoId, \
flags, trace_event_internal::kNoId, ##__VA_ARGS__); \
} \
} while (0)

INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO is also defined in
base/trace_event/trace_event.h:

#define INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO(category_group)
\
static_assert(
\

base::trace_event::BuiltinCategories::IsAllowedCategory(category_group), \
"Unknown tracing category is used. Please register your "
\
"category in base/trace_event/builtin_categories.h");
\
constexpr const unsigned char* INTERNAL_TRACE_EVENT_UID(
\
k_category_group_enabled) =
\

base::trace_event::TraceLog::GetBuiltinCategoryEnabled(category_group); \
const unsigned char* INTERNAL_TRACE_EVENT_UID(category_group_enabled);
\
INTERNAL_TRACE_EVENT_GET_CATEGORY_INFO_MAYBE_AT_COMPILE_TIME(
\
category_group, INTERNAL_TRACE_EVENT_UID(k_category_group_enabled),
\
INTERNAL_TRACE_EVENT_UID(category_group_enabled));

Finally, inside here, it
calls base::trace_event::TraceLog::GetBuiltinCategoryEnabled(), which is
defined in base/trace_event/trace_log.h:

// Called by TRACE_EVENT* macros, don't call this directly.
// The name parameter is a category group for example:
// TRACE_EVENT0("renderer,webkit", "WebViewImpl::HandleInputEvent")
static const unsigned char* GetCategoryGroupEnabled(const char* name);
static const char* GetCategoryGroupName(
const unsigned char* category_group_enabled);
static constexpr const unsigned char* GetBuiltinCategoryEnabled(
const char* name) {
TraceCategory* builtin_category =
CategoryRegistry::GetBuiltinCategoryByName(name);
if (builtin_category)
return builtin_category->state_ptr();
return nullptr;
}

Okay, so what seems like is happening here, is that the calls like this:

TRACE_EVENT_BEGIN1("base", "ScopedBlockingCall", "blocking_type",
static_cast<int>(blocking_type));

Are passing "base" (that first var) all the way into
GetCategoryGroupEnabled, which is finding it via GetBuiltinCategoryByName()
in base/trace_event/category_registry.h, checking against the list in
INTERNAL_TRACE_LIST_BUILTIN_CATEGORIES(X)
from base/trace_event/builtin_categories.h, finding it, then returning this
as builtin_category:

(& base::trace_event::CategoryRegistry::categories_[7])

When if(builtin_category) is run (trying to check to see if we got a
buillt-in category or a nullptr, I think), thats when GCC says:

error: '((& base::trace_event::CategoryRegistry::categories_[7]) != 0)' is
not a constant expression

*****

None of the other distros that build Chromium seem to have hit this issue,
nor does it appear in any bugs I could find with upstream (but they use a
fork of clang to build, and we use gcc). I have hit the limit of my C++
knowledge here, so I'm not sure how to fix this. If you can help me, it
would be greatly appreciated. My work in progress is checked into the
master chromium branch in git.

There are quite a few security issues that this new version fixes,
including some that are being actively exploited, so timely help here is
appreciated.

Thanks,

~tom

Comments

Re: Chromium C++ help needed

By Christophe de D... at 03/12/2019 - 07:56

It looks like the compiler is not smart enough to detect that this is the address of a static object, defined at line 92744 of your preprocessed file as:

static TraceCategory categories_[kMaxCategories];

whereas it is smart enough to go through GetBuiltnCategoryByName and figure out index 7 from the name…

I tried to compile your preprocessed fragment with both clang or gcc. Interestingly:

1) Without your command-line options, I don’t see the same error.

2) With your command-line options, I see the error with gcc, but not with clang.

So I suspect a compiler bug.

Re: Chromium C++ help needed

By Gary Gatling at 03/12/2019 - 13:41

On Tue, Mar 12, 2019 at 7:57 AM Christophe de Dinechin < ... at redhat dot com>
wrote:

Could chromium be built with clang in fedora? Or does it have to be built
with gcc?

Re: Chromium C++ help needed

By Jakub Jelinek at 03/11/2019 - 13:31

On Mon, Mar 11, 2019 at 01:16:07PM -0400, Tom Callaway wrote:
Can you please provide preprocessed source + g++ command line options,
from the snippets it is hard to see what's going on.
From the description it seems maybe like:
template <int N>
struct S {
static constexpr int a[2] = { 1, 2 };
};
static_assert (&S<0>::a[1] != nullptr);

which g++ accepts for -std=c++{11,14} but rejects for -std=c++{17,2a} when
S<0>::a is an inline variable. I think we have a similar
<a href="http://gcc.gnu.org/PR89074" title="http://gcc.gnu.org/PR89074">http://gcc.gnu.org/PR89074</a> . The middle-end punts here and doesn't optimize
the != NULL to true because it is address of a comdat variable and thus it
in the end could come up from any other TU. Though perhaps in these cases
the standard gives us some guarantees.

Jakub

Re: Chromium C++ help needed

By Tom "spot" Callaway at 03/13/2019 - 10:28

I tried removing some of the compiler flags to see if I could identify what
might be triggering this, and removing "-fno-delete-null-pointer-checks"
seems to make this error vanish.

Not sure if that helps or not, but hopefully, I can get this beast building
without it.

Thanks,
Tom

Re: Chromium C++ help needed

By Jakub Jelinek at 03/13/2019 - 10:36

On Wed, Mar 13, 2019 at 10:28:29AM -0400, Tom Callaway wrote:
-fno-delete-null-pointer-checks is certainly an option that requests that
the compiler does not fold comparisons against NULL at compile time based on
assumption that objects have non-NULL address, so sure, if it has such asserts,
it is incompatible with that option, you can't have both.

Jakub

Re: Chromium C++ help needed

By Tom "spot" Callaway at 03/13/2019 - 12:49

The odd thing is that this compiler flag is hardcoded into the build by the
upstream. I'm wondering why Fedora hits this when no one else seems to.

I mean, I'm fine to disable it, because the Chromium codebase is a tomb of
horrors anyway, and if I have to sacrifice that flag to make it happy, so
be it.

~tom

Re: Chromium C++ help needed

By Jakub Jelinek at 03/13/2019 - 12:53

On Wed, Mar 13, 2019 at 12:49:35PM -0400, Tom Callaway wrote:
Most likely the flag has been added to workaround other bugs in the
codebase, but maybe those were already fixed and flag forgotten.
The flag can be used to workaround bugs like calling memcpy/memset etc.
with NULL argument (when the size is 0), or calling methods on NULL pointers
to objects etc.

Jakub

Re: Chromium C++ help needed

By Michael Catanzaro at 03/13/2019 - 13:03

On Wed, Mar 13, 2019 at 11:53 AM, Jakub Jelinek < ... at redhat dot com>
wrote:
Chromium probably does this (or, at least, did in the past). Removing
-fno-delete-null-pointer-checks could introduce Fedora-specific crashes.

Michael

Re: Chromium C++ help needed

By Tom Hughes at 03/13/2019 - 13:06

On 13/03/2019 17:03, <a href="mailto: ... at gnome dot org"> ... at gnome dot org</a> wrote:
Yes v8 (and hence chromium) were well known for triggering crashes
when gcc first started assuming that people wouldn't call methods
on null pointers.

The underlying bugs in v8 where it was doing that may well be fixed
by now of course.

Tom

Re: Chromium C++ help needed

By Tom "spot" Callaway at 03/11/2019 - 13:59

Here's a g++ invocation (Fedora 30):

g++ -MMD -MF obj/base/base/memory_pressure_listener.o.d -DUSE_SYMBOLIZE
-DV8_DEPRECATION_WARNINGS -DUSE_UDEV -DUSE_AURA=1 -DUSE_GLIB=1
-DUSE_NSS_CERTS=1 -DUSE_X11=1 -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD
-DSAFE_BROWSING_DB_LOCAL -DCHROMIUM_BUILD -D_FILE_OFFSET_BITS=64
-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D__STDC_CONSTANT_MACROS
-D__STDC_FORMAT_MACROS -D_FORTIFY_SOURCE=2 -DCOMPONENT_BUILD -DNDEBUG
-DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DBASE_IMPLEMENTATION
-DGLIB_VERSION_MAX_ALLOWED=GLIB_VERSION_2_32
-DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_26 -I../.. -Igen
-fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector
-Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__=
-funwind-tables -fPIC -pipe -pthread -m64 -march=x86-64 -Wall
-Wno-unused-local-typedefs -Wno-maybe-uninitialized
-Wno-deprecated-declarations -Wno-comments -Wno-missing-field-initializers
-Wno-unused-parameter -fno-omit-frame-pointer -g0 -fvisibility=hidden
-I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -O2 -fno-ident
-fdata-sections -ffunction-sections -std=gnu++14 -Wno-narrowing
-fno-exceptions -fno-rtti -fvisibility-inlines-hidden
-fno-delete-null-pointer-checks -c
../../base/memory/memory_pressure_listener.cc -o
obj/base/base/memory_pressure_listener.o

Preprocessed source is attached.

Re: Chromium C++ help needed

By Vascom at 03/11/2019 - 13:18

Look at chromium-vaapi build in rpmfusion.

пн, 11 мар. 2019 г., 20:17 Tom Callaway < ... at redhat dot com>:

Re: Chromium C++ help needed

By Tom "spot" Callaway at 03/11/2019 - 14:05

FWIW, I did. There is no fix there.

~tom